fix(release): gate breaking API changes and bad release notes before the NuGet push - #326
Merged
Merged
Conversation
…the NuGet push Two guards this repo needed were on the wrong side of the irreversible `dotnet nuget push`. Package validation. Six packages shipped on a tag with no ApiCompat / PackageValidation / public-API-baseline check anywhere in the repo — in a project that already needed a hand-written TypeForwarders.cs to survive one assembly split. Deleting a public member produced a green CI run and a silently breaking package. `EnablePackageValidation` plus a pinned baseline now fails `pack` on any break, across all three TFMs. The enablement lives in a new src/Directory.Build.targets rather than in Directory.Build.props, because props is imported before the project body: at that point IsPackable is unset on every project, so the baseline PackageDownload lands on the test / benchmark / fuzz / AOT-smoke projects too and restore dies with "Invalid framework identifier ''". Targets is imported after the project body, where IsPackable=false is visible. Release notes. The CHANGELOG extraction and its only check lived in `github-release`, which needs `deploy` — so an over-long body failed after all six packages were already public. `## [1.5.0]` is 183,991 bytes against GitHub's ~125k cap, so this is a failure the repo has actually hit. The extraction moves into `build`, gains a body-size assertion, and is handed forward as an artifact so the release body is exactly what passed the gate. The extraction is pure shell and therefore outside `dotnet test`, so it gets its own test script and a `release-gates` CI job on every PR. The oversized-section case is the regression test: it fails against the old behaviour. Closes #315.
Coverage
|
Nothing in the PR matrix packs, so package validation and validate-packages.ps1 only ran on the nightly preview and on the release itself. That leaves a breaking API change discoverable a night late at best and after an irreversible push at worst. The release-gates job now does a full release dry run: extract-release-notes self-test, pack (which is what runs the baseline comparison), then validate-packages.ps1.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens Celerity’s release pipeline by moving two critical “fail-fast” checks to occur before the irreversible NuGet push: (1) MSBuild package/API compatibility validation during dotnet pack, and (2) CHANGELOG-derived release-notes extraction + size validation ahead of publish, with the validated notes passed forward as an artifact.
Changes:
- Add MSBuild
EnablePackageValidationwith a pinned baseline version for all packable projects, with an explicit opt-out for first releases. - Hoist release-notes extraction/validation into the
buildjob inrelease.yml, upload the validated notes as an artifact, and consume it ingithub-release. - Add CI coverage for the release-notes extraction script, plus
.gitattributesto enforce LF line endings for*.sh.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/Directory.Build.targets |
Enables package validation only for packable projects (post-project import), avoiding restore failures for dev/test tooling projects. |
src/Directory.Build.props |
Adds a single baseline version property and documents the baseline bump ritual. |
.github/workflows/release.yml |
Moves CHANGELOG extraction/validation before packaging/publish and passes validated release notes via artifact to the release job. |
.github/scripts/extract-release-notes.sh |
New script to extract a tagged version section and enforce a size guard for GitHub release body limits. |
.github/scripts/test-extract-release-notes.sh |
Regression tests for the extraction script, executed in CI. |
.github/workflows/ci.yml |
Adds a release-gates job to run the shell-script regression tests on every PR. |
docs/testing.md |
Documents the new release gates and how to run/verify them locally. |
CONTRIBUTING.md |
Updates the release procedure to include the baseline bump and documents package validation + release-notes validation. |
CHANGELOG.md |
Adds two “Fixed” entries under [Unreleased] for the new pre-publish guards. |
ROADMAP.md |
Marks #315 pipeline-integrity bullets as done. |
.gitattributes |
Forces LF endings for *.sh to avoid CI runner shebang failures. |
…ives in Addresses Copilot review on #326: the gates table listed both Directory.Build.props and Directory.Build.targets for EnablePackageValidation, but the switch is only in targets — props carries the baseline version. Split the two out and link both files. Also gitignore release-notes.md, the local output of extract-release-notes.sh when previewing a release body.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
docs/testing.md:115
- The “Package validation” row implies
EnablePackageValidationis set insrc/Directory.Build.props, but the implementation setsEnablePackageValidationinsrc/Directory.Build.targets(with only the baseline version property in props). Tweaking this wording avoids sending readers to the wrong file when debugging a validation failure.
| Package validation | Any breaking public-API change against the last published version of that package, on any TFM. | `dotnet pack`. The switch is `EnablePackageValidation` in [`src/Directory.Build.targets`](../src/Directory.Build.targets); the baseline version it compares against is `CelerityPackageValidationBaseline` in [`src/Directory.Build.props`](../src/Directory.Build.props). |
…release' Inserting the Package validation section pushed that sentence under the wrong heading, where it read as a note about validation. Moved it back above the new heading and pointed the section at the release-gates job.
Benchmarks51 regressions Highlights
Collections (488)
Hashers (111)
Same-runner A/B (sharded 8-way): main ( |
…-gates Conflicts were all additive-vs-additive, kept both sides: - .github/workflows/ci.yml — main's `dashboard-coverage` job (#301) and this branch's `release-gates` job landed in the same slot after `build-and-test`. - CHANGELOG.md — main's two dashboard entries and this branch's two release-gate entries both appended to [Unreleased] > Fixed. - ROADMAP.md — the pipeline-integrity list: three bullets flipped to `done` here, a fourth (the dashboard guard) added and flipped on main. Package validation still passes against the 2.4.0 baseline: #311's new surface (ISpanHashProvider, the span-keyed overloads, StringInternTable) is purely additive, so the gate this branch introduces stays green on the merged tree.
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
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.
Closes #315.
Two guards this repo needed were on the wrong side of the irreversible
dotnet nuget push. Both are now in front of it.(a) Package validation — the binary-compatibility gate
ApiCompat/PackageValidation/ public-API-baseline tooling did not exist anywhere in the repo. Six packages published on a tag, in a project that already needed a hand-writtenTypeForwarders.csto survive one assembly split — so deleting a public member, narrowing a parameter type, or dropping a forwarder produced a green CI run and a silently breaking package, with no test that could catch it because it is not a behavioural failure.EnablePackageValidation+ a pinned baseline now downloads each package's published predecessor atpacktime and fails the build on any break, across every TFM.Why the enablement lives in a new
src/Directory.Build.targetsand not inDirectory.Build.props: props is imported before the project body, so at that pointIsPackableis unset on every project. Enabling validation there puts a baselinePackageDownloadon the test / benchmark / fuzz / AOT-smoke projects too, which have noPackageIdto resolve it against, and restore dies witherror : Invalid framework identifier ''.Directory.Build.targetsis imported after the project body, where each dev project'sIsPackable=falseis already visible. The baseline version stays in props as a single property to bump.Two escape hatches, both deliberate rather than silent:
dotnet pack -p:ApiCompatGenerateSuppressionFile=truewrites aCompatibilitySuppressions.xmlnext to the.csproj; commit it with a comment per entry so the break is reviewed in the PR.<CelerityNoPublishedBaseline>true</CelerityNoPublishedBaseline>for one release, then drops it. This is whatCelerity.Sorting(New package: Celerity.Sorting — RadixSort, CountingSort, PartialSort over primitive keys #309) will need.(b) The release-notes check ran after the packages were live
release.yml's job graph wasbuild→deploy→github-release, with the CHANGELOG extraction and its only check ([ ! -s release-notes.md ]) insidegithub-release. An over-long body therefore failed after all six packages were already on NuGet.org, leaving a half-shipped release recoverable only by hand from the nuget artifact.This is not hypothetical:
## [1.5.0]measures 183,991 bytes against GitHub's ~125k release-body cap.The extraction moves into
build(before the build even starts — fail fast), gains a body-size assertion, and is uploaded as arelease-notesartifact thatgithub-releasedownloads instead of re-extracting, so the published body is exactly the one that passed the gate.github-releasekeepsneeds: [ deploy ]. Both new steps are guarded bystartsWith(github.ref, 'refs/tags/v'), matchinggithub-release, so aworkflow_dispatchrun from a branch is unaffected.Dry-run evidence
A deliberate breaking change fails
pack—AbuseTracker.EstimateDistinctKeystemporarily madeinternal, then reverted:mainis clean against the 2.4.0 baseline — full solution pack, exit 0, all six packages produced. So the gate is being switched on from a green state; no suppression file is needed today.The release-notes gate, against the real CHANGELOG:
The gates now run on every PR, not just at release
Nothing in the PR matrix packed, so both gates lived only on the release path (plus the nightly preview). A breaking API change was therefore discoverable a night late at best, and after an irreversible push at worst — which is the exact shape of the bug being fixed. The new
release-gatesjob inci.ymlis a release dry run on every PR: the extraction self-test, thendotnet pack(which is what runs the baseline comparison), thenvalidate-packages.ps1.validate-packages.ps1also gains per-PR coverage it did not have before.A comment in
nightly-preview.ymlnotes the same new failure mode there, so a red nightly on an API break is not a mystery.Regression test
The extraction is pure shell, so it sits outside
dotnet testand the coverage gate..github/scripts/test-extract-release-notes.shcovers it — happy path, section bounded by the next## [heading, missing section, oversized section, no partial file left behind on failure, missing changelog, missing argument — and runs as a newrelease-gatesjob inci.ymlon every PR.It is a genuine regression test, not a restatement: with the size assertion neutered to simulate pre-fix behaviour it goes red.
Package-list sync
Confirmed both halves cover all six packages.
validate-packages.ps1lists them explicitly and errors on a missing or unexpected id, and package validation applies to every project withIsPackable != false— so a new packable project is automatically in both, and the only thing its first release needs isCelerityNoPublishedBaseline.Files changed
src/Directory.Build.props(baseline version + the bump ritual),src/Directory.Build.targets(new).github/workflows/release.yml,.github/scripts/extract-release-notes.sh(new).github/scripts/test-extract-release-notes.sh(new),.github/workflows/ci.yml(release-gatesjob: extraction self-test + pack/package-validation +validate-packages.ps1),.github/workflows/nightly-preview.yml(comment)CONTRIBUTING.md(baseline bump folded into "Cutting a release" + a "Package validation" section),docs/testing.md(TL;DR row + a "Release gates" section)[Unreleased]->Fixed, one entry per gatedone; the adjacent coverage-gate bullet was stale atplannedafter #314 shipped, so it is corrected too.gitattributes(new) — pins*.shto LF so a Windows contributor cannot commit a CRLF shebang that fails on the Linux runnersParity facets that genuinely do not apply: no new collection, public API, or hasher, so there are no dedicated
*Tests.cs/ cross-collection shared-test rows, noXxxBenchmark.csorProgram.csregistration, no dashboardCOLLECTIONSentry, and nodocs/api/*.mdsection. There is no release runbook underdocs/, so per the issue the procedure went intoCONTRIBUTING.md.Test plan
dotnet build -c Release— 0 errors.dotnet test -c Release— 4926 × 3 TFMs + 113 showcase tests, all passing, 0 failures.dotnet packon the full solution against the 2.4.0 baseline — exit 0, all six.nupkg+.snupkgproduced.public->internalbreak failspackwith CP0002 on all three TFMs (log above); change reverted../.github/scripts/test-extract-release-notes.sh— 7/7 pass; 2 fail when the size check is neutered.build-and-teston ubuntu / windows / macos,coverage(100% line + branch, six assemblies),aot-publish× net8.0/net9.0/net10.0, the 8 benchmark shards + aggregate, and the newrelease-gatesjob (which is itself the pack + package-validation + package-metadata dry run).main— no benchmark class or collection was added, sogh-pagesdata.jsand the dashboardCOLLECTIONSarrays are untouched by this PR.Note for the maintainer
The baseline bump is now a step in the release ritual:
<CelerityPackageValidationBaseline>insrc/Directory.Build.propsmust move to X.Y.Z in the same commit that promotes the CHANGELOG block. It is documented in three places (the props comment,CONTRIBUTING.md,docs/testing.md) because a stale baseline fails silently — it keeps validating against an older surface rather than erroring.🤖 Generated with Claude Code