Skip to content

Conversation

@Shane32
Copy link
Owner

@Shane32 Shane32 commented Oct 4, 2025

  • Removed Deterministic because it defaults to true
  • Removed ContinuousIntegrationBuild from Debug builds because normalization of filenames prevented ShouldMatchApproved from properly reading approval files

Summary by CodeRabbit

  • Chores
    • Updated build configuration so the continuous-integration build flag applies only to Release builds in CI, standardizing file paths in release artifacts for more consistent packages.
    • Removed the explicit deterministic build setting, aligning with default tooling behavior. As a result, artifact checksums may vary between separate builds, while end-users should see no functional changes in application behavior.

@Shane32 Shane32 self-assigned this Oct 4, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 4, 2025

📝 Walkthrough

Walkthrough

Removes Deterministic and CI build flag settings from Directory.Build.props. Introduces Conditional ContinuousIntegrationBuild=true in Directory.Build.targets for Release builds when running on GitHub Actions. No public APIs changed.

Changes

Cohort / File(s) Summary
Build props cleanup
Directory.Build.props
Removed Deterministic and conditional ContinuousIntegrationBuild property definitions.
CI flag in targets (Release-only)
Directory.Build.targets
Added property group setting ContinuousIntegrationBuild to true when Configuration == Release and environment GITHUB_ACTIONS == true.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Developer/CI
  participant MSB as MSBuild
  participant Props as Directory.Build.props
  participant Targets as Directory.Build.targets

  Dev->>MSB: Build (Configuration=Release)
  MSB->>Props: Evaluate properties
  note right of Props: No Deterministic<br/>No CI flag set here
  MSB->>Targets: Evaluate targets/properties
  alt GITHUB_ACTIONS == true AND Configuration == Release
    Targets-->>MSB: Set ContinuousIntegrationBuild=true
  else Not in Release CI
    Targets-->>MSB: Do not set ContinuousIntegrationBuild
  end
  MSB-->>Dev: Build proceeds with resolved properties
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • gfoidl

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “Fix tests & code coverage reporting” accurately conveys the primary intent of the changeset, which is to adjust build properties to restore test approval matching and coverage reporting; it is concise, specific to the effect of the changes, and does not include unnecessary detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix_tests

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
Contributor

@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: 0

🧹 Nitpick comments (1)
Directory.Build.targets (1)

14-18: LGTM! Conditional logic correctly addresses the PR objective.

The implementation properly restricts ContinuousIntegrationBuild=true to Release builds on GitHub Actions, which prevents filename normalization from breaking ShouldMatchApproved in Debug builds while maintaining proper path handling for CI releases.

The condition currently targets GitHub Actions specifically. If you use other CI environments (e.g., Azure Pipelines, GitLab CI), you may want to extend the condition to include their environment variables as well.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 84f4906 and 5b2508a.

📒 Files selected for processing (2)
  • Directory.Build.props (0 hunks)
  • Directory.Build.targets (1 hunks)
💤 Files with no reviewable changes (1)
  • Directory.Build.props
⏰ 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). (2)
  • GitHub Check: additional-tests
  • GitHub Check: build

@Shane32 Shane32 merged commit d6d594a into master Oct 4, 2025
11 checks passed
@Shane32 Shane32 deleted the fix_tests branch October 4, 2025 09:56
@Shane32 Shane32 added the ci Changes to ci/cd scripts or build infrastructure label Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Changes to ci/cd scripts or build infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants