Skip to content

docs(contributing): make the release steps run - #38

Open
michen00 wants to merge 2 commits into
mainfrom
docs/release-checklist
Open

docs(contributing): make the release steps run#38
michen00 wants to merge 2 commits into
mainfrom
docs/release-checklist

Conversation

@michen00

@michen00 michen00 commented Sep 3, 2026

Copy link
Copy Markdown
Owner

What changes, and why

The ## Releasing block, followed literally, does not work.

make bump VERSION=X.Y.Z # then commit what it wrote
make check
git tag -s vX.Y.Z -m 'vX.Y.Z'
git push origin vX.Y.Z

Squash is the only merge method the main ruleset allows, so a squash merge replaces the commit the bump was made on. Tagging before the merge names a commit that never reaches main; tagging after it requires a step the block does not have. v0.4.0 points at build: bump the version to 0.4.0 (#12) — the squashed commit, an ancestor of origin/main — so the order that actually happened is bump, land, then tag what landed. The block says that now, and gains a git status --short so the tag names a clean tree.

Three claims around it were wrong or absent.

Registry credentials. The section opened "it needs push access to the tag and credentials on two registries." Both registries authorize through trusted publishing — release.yml's pypi and crates jobs mint an OIDC token, and gh api .../actions/secrets holds no registry secret. Asking a maintainer to obtain a credential the design deliberately does not use is the wrong first instruction for the section.

The verify gate. Never mentioned, and it is the job all three publishing paths wait on — binaries, pypi and crates are each needs: [verify, build]. It re-runs both suites and a 400-seed differential fuzz at the tagged commit. It also builds and tests the Rust with --locked:

$ grep -c -- '--locked' Makefile
0
$ grep -n -- '--locked' .github/workflows/release.yml
27:      - run: cargo build --release --locked
28:      - run: cargo test --locked

So a Cargo.lock disagreeing with Cargo.toml passes make check and fails after the tag is frozen, when the remedy is the next patch number rather than a retag. Only the Cargo side is exposed — that job's Python step passes no equivalent — and the paragraph says so rather than over-claiming.

The one gate after the publishes. Nothing here said a red smoke run has any consequence. It has exactly one: alias waits on it, so @v0 stays on the previous release while the version has already shipped.

Depends on #32

The sentence "release.yml runs on v*.*.* and nothing else" is left as written because #32 deletes the workflow_dispatch trigger that made it false. If #32 does not land, that clause still needs the correction described there.

Corpus

The corpus is the specification, and both implementations answer to it. Tick what applies.

  • This changes no behavior the corpus specifies.
  • This changes what gets joined, and a case in corpus/ pins the new behavior. The case was written first and failed first.
  • The change makes the tool join more than it did. The section above says what it will not eat.

Checks

  • make check passes, or make test does and this touches no Rust.

make tidy, plus unwrap-markdown-prose-py --fail-on-change CONTRIBUTING.md since tidy skips this repository's own hook. One Markdown file.

Followed literally, the four commands either tagged a commit that
squash-merging replaces or required the direct push to `main` that this
same file rules out two sections earlier. `v0.4.0` points at the
squashed commit, `build: bump the version to 0.4.0 (#12)`, so the order
that actually happened is bump, land, then tag what landed -- and only
that order leaves the tag on an ancestor of `main`. The block says so
now, and gains a `git status --short` so the tag names a clean tree.

Three claims around it were wrong or missing.

The section opened by asking for credentials on two registries. Both
publish through trusted publishing, so the workflow mints its own
short-lived token and there is nothing to hold here or locally. Asking
a maintainer to obtain a credential the design deliberately does not
use is the wrong first instruction for the section.

`verify` was never mentioned, and it is the job every publish waits on.
It re-runs both suites and the fuzzer at the tagged commit, and it
builds and tests the Rust with `--locked`, which `make check` never
passes -- so a lockfile disagreeing with its manifest goes green
locally and red after the tag is frozen, when the remedy is the next
patch number. Only the Cargo side is exposed, because that job's Python
step passes no equivalent.

And nothing here said a red smoke run has a consequence. It has exactly
one: `alias` waits on it, so the major alias stays on the previous
release while the version has already shipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix release checklist ordering and document release gates

📝 Documentation 🐞 Bug fix 🕐 10-20 Minutes

Grey Divider

AI Description

• Corrects release ordering so signed tags reference squash-merged commits on main.
• Adds clean-tree and main synchronization checks before tagging the landed release.
• Documents trusted publishing and release gates; sole-trigger wording depends on #32.
Diagram

sequenceDiagram
  actor M as Maintainer
  participant P as Release PR
  participant G as main
  participant T as Version Tag
  participant V as Verify Job
  participant R as Publish Jobs
  participant S as Smoke Test
  participant A as v0 Alias
  M->>P: Bump check clean tree
  P->>G: Squash merge
  M->>G: Fast-forward local branch
  M->>T: Sign landed commit
  T->>V: Trigger release
  V->>R: Allow publication
  R->>S: Test published artifacts
  alt Smoke passes
    S->>A: Move alias
  else Smoke fails
    S-->>A: Keep previous alias
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Maintainer release script
  • ➕ Makes synchronization, clean-tree validation, signing, and pushing fail-fast and repeatable.
  • ➕ Reduces the chance that maintainers omit or reorder checklist commands.
  • ➖ Cannot remove the asynchronous pull-request and squash-merge boundary.
  • ➖ Must preserve local GPG signing and remain synchronized with repository rules and workflows.

Recommendation: Merge the documentation correction because it accurately reflects the current signed-tag release model and explains failure consequences that automation would not eliminate. A small maintainer script could later supplement the checklist, but it should not replace the required pull-request boundary. Ensure #32 lands first or revise the claim that version tags are the workflow's only trigger.

Files changed (1) +10 / -1

Other (1) +10 / -1
CONTRIBUTING.mdCorrect and expand the maintainer release procedure +10/-1

Correct and expand the maintainer release procedure

• Reorders the checklist so the version bump is squash-merged before the landed commit is signed and tagged, adding clean-tree and local-main synchronization checks. It also documents trusted registry publishing, the post-tag verification gate and Cargo lockfile risk, plus smoke testing's effect on the movable 'v0' alias.

CONTRIBUTING.md

@codecov-commenter

codecov-commenter commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.40%. Comparing base (97d51a6) to head (62ec35b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #38   +/-   ##
=======================================
  Coverage   87.40%   87.40%           
=======================================
  Files           3        3           
  Lines         691      691           
=======================================
  Hits          604      604           
  Misses         87       87           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@qodo-code-review

qodo-code-review Bot commented Sep 3, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Manual dispatch bypasses release sequence 🐞
Description
The documentation says a version tag is the workflow's only trigger, but workflow_dispatch can run
every publishing job from a selected branch without any tag-ref validation. A maintainer can
therefore publish an unintended commit while bypassing the documented merge-and-tag sequence.
Code

CONTRIBUTING.md[91]

+This section is the maintainer's, and what it needs is push access to the tag. Both registries authorize through trusted publishing, so the workflow mints its own short-lived token and there is no registry credential to hold here or to have locally. A tag is the whole trigger. `release.yml` runs on `v*.*.*` and nothing else, so a branch push cannot publish by accident, and there is no environment gate to catch a mistake — pushing the tag is the decision.
Evidence
The workflow declares workflow_dispatch in addition to its tag trigger. Its PyPI and crates jobs
check out the selected revision and publish without checking that github.ref is a vX.Y.Z tag,
directly contradicting the changed documentation.

.github/workflows/release.yml[6-10]
.github/workflows/release.yml[168-189]
.github/workflows/release.yml[191-209]
CONTRIBUTING.md[91-103]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The release documentation claims that only a version tag triggers publication, but `release.yml` still supports `workflow_dispatch` and its publishing jobs do not require a version-tag ref. Ensure this PR cannot merge with that contradiction.

## Issue Context
The PR declares a dependency that removes manual dispatch. Either land that dependency before these docs, remove/guard `workflow_dispatch` here, or update the documentation and workflow so manually dispatched runs cannot publish.

## Fix Focus Areas
- CONTRIBUTING.md[91-103]
- .github/workflows/release.yml[6-10]
- .github/workflows/release.yml[168-207]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 9 rules
Review mode: ⚖️ Balanced

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread CONTRIBUTING.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants