ci: gate breaking public-API changes with cargo-semver-checks#10833
Open
gustavovalverde wants to merge 6 commits into
Open
ci: gate breaking public-API changes with cargo-semver-checks#10833gustavovalverde wants to merge 6 commits into
gustavovalverde wants to merge 6 commits into
Conversation
Fail a pull request that breaks a published crate's public Rust API unless the break is declared with a conventional-commit `!` in the PR title. cargo-semver-checks flags added-field and added-variant breaks, which require a major version bump even though they only add to the API. The `!` marker is the author's declaration and the signal release-plz uses to bump the major version. Add semver-checks-result to both Mergify queue condition sets so the gate blocks merges. Document the `!` convention in CONTRIBUTING and the tool division of labor in docs/proposals/release-changelog-pipeline.md.
Contributor
Merge Protections🟢 Merge protection satisfied — ready to merge. Show 1 satisfied protection🟢 📃 Configuration Change RequirementsMergify configuration change
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a CI gate that blocks pull requests which break a published crate's public Rust API unless the break is explicitly declared with a conventional-commit ! in the PR title. It fills a gap left by release-plz no longer running semver checks, shifting break detection from release time to per-PR. The new semver-checks-result aggregator is wired into the Mergify queue conditions so the gate participates in merge blocking, with branch-protection enforcement deferred to follow-up work.
Changes:
- New
semver-checks.ymlworkflow runscargo semver-checks --workspaceagainst the PR base SHA; a detected break passes only when the PR title carries a!, otherwise it fails with guidance. path-filters.ymlgains asemverfilter andmergify.ymladdscheck-success=semver-checks-resultto both queue condition sets.- Documentation:
CONTRIBUTING.mdgains the!breaking-change convention, anddocs/proposals/release-changelog-pipeline.mdrecords the gate's design.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/semver-checks.yml |
New per-PR gate: paths-filter changes job, cargo semver-checks detection, title-based ! declaration check, and an alls-green semver-checks-result aggregator. |
.github/path-filters.yml |
Adds semver filter (Rust sources, manifests, lockfile, the workflow, and the filter file). |
.github/mergify.yml |
Requires semver-checks-result in the default and urgent queue condition sets; comment updated to "four" aggregators. |
CONTRIBUTING.md |
Documents declaring breaking changes with ! in the PR title. |
docs/proposals/release-changelog-pipeline.md |
New design doc describing the breaking-change gate and its place in the release/changelog pipeline. |
Notes for the human reviewer: the two findings concern the workflow triggers — the gate is title-driven but doesn't re-run on title edited events, and it diverges from the other three required-check workflows by omitting push/merge_group triggers, which matters for the planned merge-queue enforcement (and is non-trivial to reconcile with the title-based design).
The gate failed its own CI: the pinned nightly toolchain emitted a rustdoc JSON format that cargo-semver-checks cannot parse. Use stable, the supported and recommended toolchain, matching the sibling required-check workflows. Trigger on push and merge_group with the real check gated to pull_request, and add `edited` so a title change re-runs the gate, so the merge queue is not stalled. Compute the `!` marker in the changes job and skip the check for declared breaks, so a tooling failure is reported as itself rather than as a public API break.
…se baseline The workspace run failed two ways: it semver-checked the zebrad binary, whose build script needs test-only proto files absent from the baseline checkout, and it compared against the base SHA, which lags the merge commit and attributed unrelated main-branch changes to the PR. Run cargo-semver-checks through obi1kenobi/cargo-semver-checks-action, the way lint.yml runs cargo-deny. Exclude zebrad, check default features, check out the PR head, and use the merge-base with the base branch as the baseline so only the PR's own API delta is judged.
zebra-test has two feature-gated `expect_no_requests` methods with different return types; cargo-semver-checks resolved different ones for the workspace build and the isolated baseline build, producing a false "breaking change". Exclude the binary, test-helper, and tooling crates (zebrad, zebra-test, zebra-utils) so the gate checks only the nine library crates that downstream code depends on.
Member
Author
|
This was tested with #10837 |
gustavovalverde
added
A-devops
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.
Motivation
Nothing checks whether a pull request breaks a published crate's public Rust API. Adding a field to a struct that callers construct, or a variant to an exhaustive enum, breaks downstream builds even though it only adds to the API. The change still merges and ships as a minor release.
Solution
A per-PR job runs cargo-semver-checks through
obi1kenobi/cargo-semver-checks-action, the waylint.ymlruns cargo-deny. It checks the nine published library crates against the PR's merge-base withmain, so only the PR's own API delta is judged. On a detected break the gate passes only when the PR title carries a conventional-commit!(feat(zebra-chain)!: ...), the marker release-plz uses to bump the major version; an undeclared break fails.semver-checks-resultjoins both Mergify queue condition sets so the gate blocks merges. CONTRIBUTING documents the!convention.Tests
cargo-semver-checks flags the field #10698 added,
RegtestParameters.should_allow_unshielded_coinbase_spends, asconstructible_struct_adds_fieldrequiring a new major version. This PR edits the workflow and path-filter, so the gate runs on its own PR; on this no-op change it passes across all nine crates. actionlint passes.Follow-up Work
Add
semver-checks-resultto branch protection so GitHub blocks merges directly, not only the Mergify queue.AI Disclosure
AI tools were used: Claude designed the gate, validated cargo-semver-checks against #10698, and drafted the workflow.