feat(build): milestone 8 phase 0 - package readiness hardening - #46
Merged
Conversation
Executes ADR-0031's package-readiness bar (PLAN-0008 Phase 0) across all five packages: renames publish-preview.yaml's prerelease identifier to preview, exact-pins the Compono-family lockstep dependency in each integration package's packed .nuspec, centralizes discovery metadata (tags/release notes/titles), enables Microsoft.DotNet.PackageValidation, remaps release-drafter's breaking-change label to a minor bump so 0.x never exits by accident, and adds a new locally-controlled package-validation.yaml CI gate (pack + contents inspection + CS1591 enforcement + local-feed packed-consumer smoke test). Also records ADR-0031 Amendment 1: the original "exact pins for everything" policy was corrected before shipping - third-party dependencies that flow into a publishable package's own .nuspec (NSubstitute, Bogus, xunit.v3.extensibility.core) now get a deliberate tested range instead of a blanket exact pin, per user direction. Only the internal Compono-family lockstep pin stays exact. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 624c913d35
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- release-drafter.yml: major's version-resolver must be an object with labels (matching minor/patch's shape), not a bare array - the array form fails Release Drafter's own config validation. - package-validation.yaml: only a confirmed 404 from nuget.org's flatcontainer index means "never published, baseline inert" - a rate-limit, 5xx, or transport failure now fails the job instead of silently falling through to an empty baseline and skipping the API-compatibility check. - inspect-packed-nupkgs.sh: asserts the packed .nuspec's <title>, <tags>, and <releaseNotes> content, not just that the .nuspec file exists - closes the gap where the discovery-metadata gate could stay green even if one of those got misspelled or dropped. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3924712713
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- assert_exact_pin_dependency now compares the Compono dependency pin against the integration package's own <version>, not just a ^\[.*\]$ shape regex - the old regex would silently accept a stale pin ([0.9.0] inside a 1.0.0 package) or an inclusive range ([0.9.0,1.0.0]), both bracket-shaped but not what lockstep requires. - Replaced the .pdb/obj/ denylist with assert_exact_file_listing, an allowlist of every path a clean pack actually produces (lib/README/ icon/nuspec/OPC plumbing, plus Compono.Generators.dll for Compono) - a stray artifact that isn't a .pdb and isn't under obj/ (a leaked test DLL, a stray .deps.json) previously passed silently. Verified locally both directions for each check: a real pack passes cleanly, and a hand-constructed regression (a self-inconsistent version/pin, and a leaked extra file) is caught. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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
Executes ADR-0031's package-readiness bar (PLAN-0008 Phase 0) against all five packages (
Compono,Compono.XunitV3,Compono.NSubstitute,Compono.Bogus,Compono.Generators), plus a mid-implementation correction to that ADR's dependency-pinning policy.Changes
Release pipeline / versioning
publish-preview.yaml'sprereleaseIdentifierrenamedalpha→preview..github/release-drafter.yml:breaking-changelabel now resolves to aminorbump, notmajor— a labeled breaking-change PR no longer silently exits the0.xline.Dependency pinning (ADR-0031 Amendment 1)
Compono.XunitV3/Compono.NSubstitute/Compono.Bogus) now exact-pins its packedComponodependency ([x.y.z]) via aPinProjectReferenceVersionsExactMSBuild target that rewrites the SDK's own_ProjectReferencesWithVersionsitem —<ProjectReference>stays for local dev, only the version range written into the.nuspecchanges..nuspec(NSubstitute,Bogus,xunit.v3.extensibility.core) now get a deliberate tested range instead ([6.0.0, 7.0.0),[35.6.5, 36.0.0),[3.2.2, 4.0.0)) — tested minimum, exclusive next-untested-major upper bound. Everything else inDirectory.Packages.props(build-only/test-only tooling that never reaches a consumer's.nuspec) is unchanged.Package metadata
Directory.Build.props: centralizedPackageTags/PackageReleaseNotes(one value for all five packages) andEnablePackageValidation=true(no static baseline — resolved per-PR by the new CI job below).<Title>.New CI gate —
.github/workflows/package-validation.yamlopened/synchronize/reopened/labeled/unlabeled(must see label changes, sincerelease-drafter.yamlappliesbreaking-changein its own separately-triggered run).-p:PackageValidationBaselineVersion=<prior>(skipped only onbreaking-change-labeled PRs).CS1591(missing XML doc comment) as a build error for the four publishable packages..github/scripts/inspect-packed-nupkgs.sh: asserts each.nupkg's contents (lib/README/icon per TFM, no stray artifacts,Compono.Generators.dllembedded inCompono'sanalyzers/dotnet/cs, and the exact-pin/tested-range dependency versions above).test/Compono.XunitV3.SampleTests' ownPackToLocalFeedrestore, filtered past its one deliberately-failing test class via MTP's--filter-not-class(its VSTest-style--filterproduces zero matches against an MTP v2 host — caught during verification).Validation
dotnet build/dotnet testonCompono.slnx: 854/854 tests pass.dotnet packall four publishable packages locally;inspect-packed-nupkgs.shpasses against the output (contents, exact-pin, and tested-range checks all green).dotnet build -p:WarningsAsErrors=CS1591for all four publishable packages: 0 warnings, 0 errors — every public member already has a doc comment.dotnet pack -p:PackageValidationBaselineVersion=0.1.0-alpha.35(the real currently-latest publishedComponoversion) against a locally-built0.9.0— validation runs and finds no incompatibility.dotnet test test/Compono.XunitV3.SampleTests -- --filter-not-class "Compono.XunitV3.SampleTests.FailingCompositionTests"— 16/16 pass, restoring all four packages from a real packed local feed. This also serves as thePrivateAssets/analyzer-transitivity verification task (real generated plans composed through packages consumed only viaPackageReference, neverProjectReference).Directory.Packages.props's current set: all MIT/BSD/Apache-2.0, no copyleft.Notes for Reviewers
The dependency-pinning correction (Amendment 1) happened mid-implementation, after the original blanket exact-pin approach was already coded and verified — see the plan's Phase 0 Notes entry for the full account. Flagging in case the amendment's scope (internal lockstep stays exact; only external deps become ranged) doesn't match what you had in mind.