chore: arm the warnings-as-errors gate; zero-warning build - #155
Merged
Conversation
…rnings The TreatWarningsAsErrors gate keyed on ContinuousIntegrationBuild, which CI never set (GitHub Actions only sets CI=true) — so CI built green with ~236 unique analyzer warnings. Directory.Build.props now maps CI=true to ContinuousIntegrationBuild=true, and the warning debt is cleared: Fixed in code: - CA1305: TypeCache Convert.ToInt32 with invariant culture - CA1307: ordinal Contains in Guard.Path; ordinal Replace in tests - CA2007: ConfigureAwait(false) scope pattern for await-using in Testing MSBuild helpers - CA1063: full Dispose(bool) pattern on SolutionRefactoringTest - CA1859: concrete return types on three private helpers - CA1822: removed dead Dispose on private NonGenericEnumerator - RS2008: pragma for the test-only descriptor - CA1849: pragma x2 (CancelAsync unavailable on netstandard2.0) - xUnit1051 x6: TestContext cancellation tokens in recent tests (caught by the newly armed gate — errors, not warnings, proving the gate works) Configured off in .editorconfig, each with a written justification: CA1062 (redundant with enforced NRT), CA1815, CA1031, CA1034, CA1054/CA1055, CA1028, CA1024, CA1710/CA1711/CA1721, CA2225, CA1819. CI=true clean rebuild: 0 warnings, 0 errors. 215 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (15)
Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting Summary by CodeRabbit
WalkthroughChanges include a CI-driven warnings-as-errors toggle in Directory.Build.props, ConfigureAwait-based async disposal scoping in MSBuild helper/test-base classes, a Dispose(bool) pattern addition in SolutionRefactoringTest, CA1849 suppressions, return-type tightening on several utility methods, ordinal-comparison and invariant-culture fixes in Guard.Path and TypeCache, and corresponding test updates for culture/cancellation-token consistency. ChangesBuild config, async disposal, and code cleanups
Estimated code review effort: 2 (Simple) | ~12 minutes Possibly related PRs
Suggested labels: Comment |
This was referenced Jul 6, 2026
Merged
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 6, 2026
….2.35 round-trip surface) (#157) Arc A (#155 → 2.2.34): warnings-as-errors gate armed, zero-warning build. Arc B (#156 → 2.2.35): GetFullyQualifiedMetadataName + GetFullyQualifiedName. Both indexed on nuget.org; qyl bumped 2.2.33 → 2.2.35 in qyl#494. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The gate was toothless:
TreatWarningsAsErrorskeys onContinuousIntegrationBuild, which the workflow never sets (onlyCI=true) — so CI has been building green with ~236 unique analyzer warnings. This PR arms the gate and clears the debt so it can stay armed.1. Arm the gate
Directory.Build.propsmapsCI=true→ContinuousIntegrationBuild=true. From this PR on, any new warning fails CI.2. Fix the real ones in code
CA1305 (invariant culture in
TypeCache), CA1307 (ordinalContains/Replace), CA2007 (ConfigureAwait scope pattern forawait usingin Testing MSBuild helpers ×5), CA1063 (properDispose(bool)onSolutionRefactoringTest), CA1859 (concrete return types on 3 private helpers), CA1822 (deadDisposeremoved), RS2008 (test-only descriptor pragma), CA1849 (pragma ×2 —CancelAsyncdoesn't exist on netstandard2.0).Proof the gate works: arming it immediately surfaced 6
xUnit1051violations (missingTestContextcancellation tokens) in tests added during 2.2.30–2.2.33 — as errors. All fixed.3. Categorical rules configured off — each with a written justification in
.editorconfigCA1062 (redundant with enforced NRT on ~390 sites; Roslyn inputs non-null by contract), CA1815 (scope/builder/enumerator structs), CA1031 (deliberate exception-to-data helpers), CA1034 (zero-alloc enumerator pattern), CA1054/CA1055 (opaque OTel schema URLs), CA1028 (wire-format byte enums), CA1024 + naming rules CA1710/CA1711/CA1721 (shipped API stability), CA2225 (documented
As*alternates exist), CA1819 (snapshot arrays by design). New warning IDs not on this list still fail the build.Verification
CI=trueclean rebuild: 0 warnings, 0 errors (was 236 unique / 467 total warnings)Status: complete-and-verified. Touches
src/**/tests/**→ publishes as 2.2.34 via auto-bump.🤖 Generated with Claude Code