Repository navigation
build: treat warnings as errors, drop GeneratePackageOnBuild - #7
Merged
Merged
Conversation
This was referenced Aug 16, 2026
Nothing policed warnings: a Release build of the solution carried 34 EF1001 and one CS0618 without anyone seeing them, and CS1591 on the shipping projects' public surface — the .xml a consumer's IntelliSense reads — could accumulate the same way (the sibling repository reached 64 before turning this on). TreatWarningsAsErrors is now set in Directory.Build.props, with NU1901-NU1904 kept as warnings: they report upstream advisories, so as errors they would break unrelated CI runs at unpredictable moments, and they must stay visible rather than be suppressed. The 34 EF1001: 32 are EF's analyzer reading this repository's own CodoMetis.ValueRanges.EntityFrameworkCore.PostgreSQL.Internal namespace as EF-internal when the NodaTime satellite consumes it — the analyzer keys on the `EntityFrameworkCore.*.Internal` shape, not on an attribute. That warning is aimed at consumers; the satellite is the same codebase. The other two are Npgsql's PgNewArrayExpression in the value-set translator, which has no public equivalent. All four files acknowledge it with a file-level pragma next to the reason, never repo-wide, so the next accidental one still surfaces. The CS0618 was Testcontainers' obsolete parameterless PostgreSqlBuilder(); the image now goes through the constructor. Same image, no behaviour change. GeneratePackageOnBuild is dropped for the reason the sibling repository documented: the SDK's pack targets prepend Build to GenerateNuspecDependsOn only when it is not set, so plain `dotnet pack` silently behaved as --no-build and failed with NU5026 on a clean checkout — CLAUDE.md had been carrying the workaround. PackagingConventionTests now keeps it out. Verified: the new convention test fails against the previous Directory.Build.props and passes now; a full --no-incremental Release build of the solution is warning-free; plain `dotnet pack` from a clean bin/ produces the package; the whole suite passes, integration included (Docker). Co-Authored-By: Claude Fable 5 <[email protected]>
CaffeinatedCoder
force-pushed
the
build/warnings-as-errors
branch
from
August 16, 2026 12:07
b89ff04 to
57334b3
Compare
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.
Why
Nothing policed warnings: a Release build of the solution carried 34 × EF1001 and 1 × CS0618 without anyone seeing them, and CS1591 on the shipping projects' public surface — the
.xmla consumer's IntelliSense reads — could accumulate the same way (EFCore.ComplexIndexes reached 64 before turning this on in 5.0.3). Separately,GeneratePackageOnBuild=trueis the trap the sibling repo documented and CLAUDE.md here was carrying the workaround for.What
Directory.Build.props:TreatWarningsAsErrors=true, withNU1901–NU1904kept as warnings (upstream advisories must not break unrelated runs, and must stay visible). Same carve-out and reasoning as EFCore.ComplexIndexes.CodoMetis.ValueRanges.EntityFrameworkCore.PostgreSQL.Internalnamespace as EF-internal when the NodaTime satellite consumes it — the analyzer keys on theEntityFrameworkCore.*.Internalnamespace shape, not on an attribute. That warning is aimed at consumers; the satellite is the same codebase. The other 2 are Npgsql'sPgNewArrayExpressioninValueSetsMethodCallTranslator, which has no public equivalent. All four files acknowledge it with a file-level pragma next to the reason — never repo-wide, so the next accidental one still surfaces.PostgreSqlBuilder(); the image now goes through the constructor. Same image, no behaviour change.GeneratePackageOnBuilddropped, with the explanation from the sibling repo in the props file;PackagingConventionTests.SharedBuildProperties_DoNotSetGeneratePackageOnBuildkeeps it out. CLAUDE.md's pack command is now plaindotnet pack -c Release.Verified
Directory.Build.propsand passes now.--no-incrementalRelease build of the solution: warning-free.dotnet packfrom a cleanbin/produces the package (the cold-checkout case).🤖 Generated with Claude Code