Skip to content

Conversation

@0xced
Copy link
Contributor

@0xced 0xced commented Oct 23, 2025

What does this PR do?

Run the continuous delivery job only from the testcontainers/testcontainers-dotnet repository.

The secrets required for the cd job to run successfully won't be configured in the forks anyway.

Why is it important?

The continuous delivery job has no reason to be run from forks. It will just end up with errors.

Related issues

N/A

@netlify
Copy link

netlify bot commented Oct 23, 2025

Deploy Preview for testcontainers-dotnet ready!

Name Link
🔨 Latest commit cd0d38a
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-dotnet/deploys/68fe345cfdfc530008c87527
😎 Deploy Preview https://deploy-preview-1559--testcontainers-dotnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link

coderabbitai bot commented Oct 23, 2025

Summary by CodeRabbit

Release Notes

  • Version

    • Version updated to 4.9.0
  • Chores

    • Strengthened CI/CD pipeline validation with enhanced repository and branch requirement checks to improve build reliability and security
    • Reorganized workflow configuration structure for improved project maintainability

Walkthrough

Require the CI/CD workflow to run only when both the repository matches testcontainers/testcontainers-dotnet and the branch is develop or main; add a new Solution Items project "Workflows" that includes three GitHub Actions workflow files; bump Version in Directory.Build.props from 4.8.1 to 4.9.0.

Changes

Cohort / File(s) Summary
CI/CD Workflow
\.github/workflows/cicd.yml
Reworked the workflow condition to require both repository equality (testcontainers/testcontainers-dotnet) and branch membership ("develop", "main") before executing deployment jobs.
Solution Items / Workflows
Testcontainers.sln, \.github/workflows/...
Added a new solution-level project Workflows (Solution Items) and added three workflow files (.github/workflows/cicd.yml, codeql-analysis.yml, test-report.yml); updated GlobalSection(NestedProjects) to nest Workflows under Solution Items.
Version bump
Directory.Build.props
Updated Version from 4.8.1 to 4.9.0 (AssemblyVersion/FileVersion remain bound to $(Version)).

Sequence Diagram(s)

sequenceDiagram
    participant Dev as Developer (push/PR)
    participant GH as GitHub
    participant WF as cicd.yml (workflow)

    Dev->>GH: push / open PR
    GH->>WF: evaluate workflow triggers
    note right of WF #DDEBF7: Condition: repo == "testcontainers/testcontainers-dotnet"\nand branch in ["develop","main"]
    alt Condition met
        WF->>WF: run deployment jobs
        WF-->GH: report success/failure
    else Condition not met
        WF-->GH: skip deployment (no job run)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Pay attention to the exact condition expression in .github/workflows/cicd.yml (logic and quoting).
  • Verify Testcontainers.sln nested-project GUIDs and solution structure for Visual Studio compatibility.
  • Confirm Directory.Build.props version bump does not require downstream package/versioning changes.

Poem

🐰 I nudged the workflows, tidy and bright,
Repo and branch now guard the night.
A new "Workflows" burrow snugly in sight,
Version hopped up, everything just right. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "chore: Make the continuous delivery job fork-friendly" directly aligns with the primary objective of the changeset. According to the raw summary and PR objectives, the main change modifies the CI configuration in .github/workflows/cicd.yml to restrict the continuous delivery job to run only from the official testcontainers/testcontainers-dotnet repository. The title accurately and concisely captures this intent, using clear language that a teammate scanning the history would immediately understand as being about preventing CD job execution in forks.
Description Check ✅ Passed The pull request description includes both mandatory sections from the template. The "What does this PR do?" section clearly explains the change: restricting the CD job to the official repository. The "Why is it important?" section provides the rationale: preventing errors in forks where secrets aren't configured. The "Related issues" section is appropriately marked as "N/A." While the "How to test this PR" and "Follow-ups" sections are not included, these are optional or recommended components, and the description is sufficiently complete with the mandatory information filled in comprehensively.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 293d43f and cd0d38a.

📒 Files selected for processing (2)
  • Directory.Build.props (1 hunks)
  • Testcontainers.sln (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
  • GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
  • GitHub Check: analyze (csharp)
🔇 Additional comments (3)
Directory.Build.props (1)

5-5: Clarify the version bump intent.

The version is updated from 4.8.1 to 4.9.0, but this change is not mentioned in the PR description. The PR's stated objective is making the CD job fork-friendly, which typically wouldn't require a version bump.

Is this version bump intentional as part of release preparation, or should it be reverted/moved to a separate PR?

Testcontainers.sln (2)

12-18: LGTM - Good developer experience improvement.

The addition of workflow files to the solution improves discoverability and enables editing workflows directly from Visual Studio. The project structure is correct.


766-766: LGTM - Proper nesting configuration.

The Workflows folder is correctly nested under the Solution Items container.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2e32476 and c027999.

📒 Files selected for processing (2)
  • .github/workflows/cicd.yml (1 hunks)
  • Testcontainers.sln (2 hunks)
🔇 Additional comments (1)
.github/workflows/cicd.yml (1)

92-92: LGTM! Repository check correctly prevents fork execution.

The CD job now requires both the correct repository and branch, preventing failures on forks where secrets are unavailable.

That is, run it only from the testcontainers/testcontainers-dotnet repository.

The secrets required for the `cd` job to run successfully won't be configured in the forks anyway.
@0xced 0xced force-pushed the feature/cd-fork-friendly branch from c027999 to 293d43f Compare October 23, 2025 17:06
@HofmeisterAn HofmeisterAn added the chore A change that doesn't impact the existing functionality, e.g. internal refactorings or cleanups label Oct 26, 2025
@HofmeisterAn HofmeisterAn changed the base branch from develop to main October 26, 2025 14:47
@HofmeisterAn HofmeisterAn changed the base branch from main to develop October 26, 2025 14:47
@HofmeisterAn HofmeisterAn merged commit 4a72cb3 into testcontainers:develop Oct 26, 2025
74 checks passed
@0xced 0xced deleted the feature/cd-fork-friendly branch October 26, 2025 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore A change that doesn't impact the existing functionality, e.g. internal refactorings or cleanups

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants