fix: harden the release workflow - #3283
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3283 +/- ##
==========================================
+ Coverage 90.28% 90.34% +0.05%
==========================================
Files 251 250 -1
Lines 88562 89131 +569
Branches 88562 89131 +569
==========================================
+ Hits 79960 80524 +564
+ Misses 5718 5672 -46
- Partials 2884 2935 +51 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Benchmark results: ✅ PassSummary: 🚀 0 · ✅ 10 · ☑️ 4 · 🚧 1 · ❌ 0 Per-benchmark results (15 rows)
Legend: 🚀 ≥1.15x faster · ✅ faster or unchanged · ☑️ ≤1.03x slower · 🚧 1.03x-1.15x slower · ❌ ≥1.15x slower |
nicklan
left a comment
nicklan
left a comment
There was a problem hiding this comment.
nice, this will help a lot I think. left some comments
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Review (draft - human review required)
Show review
No blocking issues. This is a well-scoped hardening of the release tooling: the subcommand dispatcher, the fail-closed changelog verification, the semver-only tag pattern, and the backup/restore on a failed refresh all hold up. Two candidate correctness concerns (pipeline failure propagation through the latest_kernel_release_tag command substitution, and the single-PR-token match in verify_release_changelog) were checked and dismissed: bash command substitutions inherit pipefail, and the last-token match deliberately mirrors what cliff.toml renders. The notes below are non-blocking.
Summary
The change is sound and no blocking issues were found; the notes above are maintainability and coverage suggestions. Coverage was reduced: the maintainer-claude-reviewer pass stalled and did not return after one retry, so this review reflects the maintainer-codex, architecture, test-coverage, and docs reviewers plus a disprove gate that dismissed both candidate blockers.
Automated review - workflow run
| printf -v RELEASE_CHANGELOG_HEADING '## [v%s]' "$version" | ||
| } | ||
|
|
||
| release_changelog_section() { |
There was a problem hiding this comment.
Nit1 The changelog format now has two authors. cliff.toml's Tera template owns rendering, but release.sh independently re-decodes it: the ## [v...] heading via printf/awk, the [#N]: back-reference via grep -Fq "[#$pr]:" (line 180), and cliff's last-token PR extraction via regex; release_changelog_section and strip_release_changelog_section (196-204) also encode the same section boundary twice. A template edit can silently desync the parsers, causing phantom 'missing PR' failures or a false pass. Raised by: architecture-reviewer, maintainer-codex-reviewer. Suggested fix: derive the rendered [#N]: set from the same git cliff --context output instead of re-deriving cliff's link rule, and share one section-boundary helper between the two awk programs.
|
|
||
| release_changelog_heading() { | ||
| local version="$1" | ||
| printf -v RELEASE_CHANGELOG_HEADING '## [v%s]' "$version" |
There was a problem hiding this comment.
Nit2 release_changelog_heading mutates the file-scope RELEASE_CHANGELOG_HEADING global, while every sibling helper returns on stdout for $(...) capture. The hidden 'call, then read the global' dependency is not visible at the call site and does not compose. Raised by: architecture-reviewer. Suggested fix: release_changelog_heading() { printf '## [v%s]' "$1"; } and let callers do heading=$(release_changelog_heading "$version").
| log_warning "No prior Kernel release tag found" | ||
| return 1 | ||
| fi | ||
| if [[ "$previous_tag" == "v$version" ]]; then |
There was a problem hiding this comment.
Nit3 Several new guard branches are untested by test-release.sh: this already-tagged equal-version early return (160-163), the empty-section guard (166-169), the main() unknown-command/argument-count errors, and the handle_release_branch abort when verify_release_changelog fails (252-254). The release_changelog_subjects failure branch (171-174) is also unreachable in the harness because latest_kernel_release_tag fails first. Raised by: test-coverage-reviewer. Suggested fix: add cases that tag the release and assert this path exits 0, request a version with no section and assert failure, and drive an unknown command / missing argument.
| postprocessors = [] | ||
|
|
||
| [git] | ||
| # Only plain semver tags mark Kernel releases. Artifact-specific tags such as v0.0.1_dat must not |
There was a problem hiding this comment.
Nit4 This comment (and RELEASING.md) say only 'plain semantic-version tags such as v0.28.0' act as release boundaries, but tag_pattern also admits pre-release tags such as v0.29.0-rc.1. Wording nuance, not a functional error. Raised by: docs-reviewer. Suggested fix: say 'plain or pre-release semver tags', or tighten the pattern if pre-releases should not be boundaries.
nicklan
left a comment
nicklan
left a comment
There was a problem hiding this comment.
lgtm, although i think the nits added by the bot mostly make sense to do. thanks!
2 participants
What changes are proposed in this pull request?
Harden the Kernel release workflow:
release.shPR is open cannot be silently omitted
v0.0.1_datcannot truncate the changelogChangelog verification uses git-cliff's own filtered commit context and reachable branch tags,
runs only on
release/branches, and fails closed if its tooling cannot resolve the release range.A failed refresh restores the existing changelog.
This complements #3066, which handles independently versioned UC crates. Its suffixed per-crate
tags are deliberately excluded from Kernel changelog boundaries.
How was this change tested?
.github/scripts/test-release.shwith git-cliff 2.10.1, covering alternate-registry forwarding,artifact and divergent tags, late PRs, skipped release commits, idempotent refresh, and failure
restoration
./release.sh verify-changelogbash -nand ShellCheck 0.10.0cargo +nightly fmt -- --checkcargo doc --workspace --all-features --no-deps