feat(ci): replace release-please with git-cliff - #774
Draft
cbrgm wants to merge 7 commits into
Draft
Conversation
release-please walks the branch history in commit-date order and stops at the sha of the last release. That boundary does not hold with merge commits: the shipped v0.3.0 notes carry 64 feat/fix entries for 36 merged PRs, list the same change twice, and drop the PR links for #729 and #713. git-cliff takes a real tag range instead. Notes are built from merge commits, identified by the trailing (#<pr>), so the branch-side commits of a PR no longer appear next to the PR itself. Replaying v0.3.0-rc2..v0.3.0 through the new config yields 24 bug fixes against 24 merged fix PRs. The Release workflow is dispatched by hand and never writes to main: it computes the version, renders the notes, pushes the tag and opens a draft release. Two guards keep the merge-commit filter honest. One fails when a commit on main carries no PR number, the other refuses a release with no feat/fix/perf since the last tag, which is needed because git-cliff's no_increment_regex is a no-op in 2.13.1. The asset build moves into a reusable workflow so the release path and the manual recovery path share one definition. Dropping the Release PR also drops what it required: the GitHub App, the RELEASE_PLEASE_APP_ID and _PRIVATE_KEY credentials, and the "allow Actions to create and approve pull requests" setting. Those can be revoked.
Renders the next version and its notes into the job summary, so the pending changelog stays visible without a rolling draft release, which would either rebuild and re-sign every artefact per merge or leave stale binaries attached to the draft. Also warns as soon as a merge lands without a PR number, rather than only failing later at release time.
Both human decisions are unchanged: cut the release, then publish the draft. Records that breaking changes bump the minor while pre-1.0, and that the notes depend on the merge commit title being the PR title.
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
git-cliff exposes a merge_commit flag on the commit context, so one parser rule replaces matching every subject against a trailing (#<pr>). Better on three counts. It keys on git topology rather than on the merge button's title setting, so the notes no longer depend on a repo setting that can be changed. It is exact: replaying v0.3.0-rc2..v0.3.0 now gives 12 features against the 12 feat PRs that merged, where the suffix match gave 13. The extra was da5f12f, a single-parent squash commit sitting on a branch, which the suffix cannot tell apart from a real merge. The trade is that squash and rebase merges leave no merge commit and would be invisible, so the guard flips from "must carry a PR number" to "must be a merge commit" and the preview warns on the same condition. Also collapses three git-cliff invocations into two: --context --bump returns the computed version and the previous tag together, and both guards now read that same filtered commit set rather than re-deriving it with git log.
Merge commit bodies are the PR description verbatim, so machine-written prose lands in footer position. A CodeRabbit summary containing a line-start "BREAKING CHANGE:" would have bumped the minor on its own. 39 of the 79 merge commits in the last release carry such a block, so strip it before parsing. A human-written BREAKING CHANGE: footer still counts. Roll the tag back when the release call fails, otherwise the tag is orphaned and a re-run trips the "tag already exists" check. Declare sha as an explicit optional dispatch input on Release Assets rather than relying on an undefined input reference resolving to empty.
Two similarly named entries in the Actions list left you picking between "Release" and "Release Assets" without anything saying which cuts a release. Renames the second to "Release (retry asset upload)", so the sidebar answers the question on its own, and spells out in both input descriptions that Release creates the tag itself. The retry workflow now also fails with a pointer to Release when the tag has no release at all, or when the release is already published, instead of failing later inside gh release upload. Adds a run-name so a run reads as "Release v0.4.0 (dry run)" in the list rather than just "Release". Deliberately not merged into a single create-or-resume workflow: resuming means the tag already exists, so there is nothing unreleased to compute a version or render notes from, and the whole job would have to branch. Two clearly named workflows are simpler than one with two modes.
zizmor at the auditor persona flagged both workflows for missing concurrency. That one is worth acting on rather than silencing: two Release runs at once would race to create the same tag, and two asset builds would race on the same upload. Neither cancels in flight, since a run may already have tagged and drafted. Also moves the permission comments inline, which is the form zizmor recognises and matches what the other workflows here already do for id-token.
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
2 participants
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.
What
cliff.toml+ a dispatchedReleaseworkflow replace release-pleaseRelease Assetsworkflow, shared with the manual recovery runChangelog Previewputs the pending notes in the job summary on every push to mainBoth human steps stay as they are: cut the release, then publish the draft. docker / helm / docs still fire on
release: published, untouched.Why
release-please walks history in commit-date order and stops at the sha of the last release. fine with squash merges since main is linear, not with merge commits. you can see it in the v0.3.0 notes we just shipped -> 64 feat/fix entries for 36 merged PRs,
**dev:** make demoin there twice, #729 and #713 missing their PR links.git-cliff takes a real tag range instead, so commit dates cant move the boundary. notes come from the merge commits on main, matched on git-cliff's
merge_commitflag, so everything on the branch side of a merge is dropped. squash and rebase merges leave no merge commit and would be invisible -> the release fails and the preview warns when something lands on main that isnt a merge. second guard refuses a release with no feat/fix/perf, because git-cliff'sno_increment_regexis a no-op in 2.13.1 (their own doc example returns0.1.1where it says0.1.0).nothing writes to main anymore -> the App,
RELEASE_PLEASE_APP_ID/_PRIVATE_KEYand "allow Actions to create and approve PRs" can go once this merges.Testing
Real git-cliff 2.13.1 over the range we already shipped:
24 against the 24 fix PRs that actually merged, #729 / #713 back in. Features gives 12 against 12, so the filter is exact.
Bump matrix on synthetic history checks out, using real merges rather than fake subjects:
fix-> patch,feat-> minor,feat!-> minor while pre-1.0. Branch side commits dont bump. Except afeat!on a branch whose PR title saysfix:, that one still gives v0.4.0 viaprotect_breaking_commits, which is the point. A squash merge with no merge commit gets caught by the guard.Both guards refuse against main as it stands, only a
ci:commit since v0.3.0 where git-cliff alone would have said v0.3.1. actionlint clean.Edge cases worth knowing, all covered in the last commit. Merge commit bodies are the PR description verbatim, so a line-start
BREAKING CHANGE:in there used to bump the minor on its own. 39 of the 79 merge commits in the last release carry a CodeRabbit block, so that block gets stripped before parsing now, a human written footer still counts. The tag rolls back if the release call fails, otherwise a re-run trips "tag already exists". Also checked:default_workflow_permissionsisreadon this repo, job levelcontents: writeescalates over it fine, and no ruleset covers tags.Cant test tag + draft creation before this merges. Run
Releasewithdry_runchecked first, it renders and creates nothing.Follow ups
RELEASE_PLEASE_APP_ID/_PRIVATE_KEYand the App installChecklist
releasing.md)Closes opendefensecloud/odd-internal#78