ci: feed the generated CHANGELOG.md to the release body, and gate its drift - #529
Conversation
The repository had no changelog at all -- release history lived only in git tags and GitHub Releases. - `CHANGELOG.rst` at the repository root, 632 lines, covering 0.13.0 through a new `1.5.0 -- unreleased` section - `docs/source/changelog.rst`, a stub that only does `.. include:: ../../CHANGELOG.rst`, so no content is duplicated - `changelog` added to the second toctree in `docs/source/index.rst` - `pyproject.toml` `[project.urls].changelog` repointed from the Releases page to the rendered page Root rather than `docs/` because `MANIFEST.in` carries both `global-include *.rst` and `prune docs`: a changelog kept only under `docs/` would be missing from every sdist, which is where packagers read it. A copy in both places would drift. The stub deliberately has no title of its own -- measured, one carrying its own title renders two `<h1>` elements. `:start-after:` was rejected for silently dropping the file's intro line. Nitpicky build warning count is unchanged at 1166 with an identical warning set, and the file parses clean under plain docutils, so the sdist copy survives a PyPI-style render.
Replaces the single root CHANGELOG.rst with the layout the release chain actually needs: a Markdown CHANGELOG.md at the root holding only the newest version's entry, and the full history under docs/ as .rst wired into Sphinx. - docs/source/changelog/<version>.rst: 37 per-version entries, split out of the 632-line root file. Concatenating the preamble and all 37 in toctree order reproduces CHANGELOG.rst byte-for-byte (32,671 bytes, 632 lines). - docs/source/changelog.rst: title, preamble and a 37-entry toctree, already reachable from docs/source/index.rst:33. - CHANGELOG.md: generated, not maintained. 40 lines of GFM holding the 1.5.0 entry, for the Create Release body and the sdist surface, both of which read Markdown rather than reStructuredText. - util/changelog_md.py: the generator, plus a --check mode that exits non-zero when the committed CHANGELOG.md has drifted from its source entry. "Newest" comes from the toctree's first line rather than a version sort, since PEP 440 ordering and lexical ordering disagree on 1.5.0b3 against 1.5.0 against 1.10.0. A residual-markup guard of 13 patterns refuses to emit when any unconverted RST survives, so nothing malformed can reach a published release. - MANIFEST.in: include CHANGELOG.md explicitly, since global-include *.rst does not match it, and ship the per-version entries. CHANGELOG.md is Markdown by deliberate exception to the .rst convention: its consumers are GitHub's release renderer and PyPI, neither of which reads RST. 34 tests, 37 subtests. Sphinx warning set unchanged against main. sdist ships CHANGELOG.md and all 37 entries, and twine check passes.
The trailer explained which `docs/source/changelog/<version>.rst` the entry came from. CHANGELOG.md is read on a GitHub release page and on PyPI, where the reader has no repository in front of them, so that half was noise to its only two audiences while the link is useful to both. Regenerated accordingly; `--check` confirms the committed file is in step.
… drift - The release step set `generate_release_notes: true` with no body, so the hand-written changelog reached no reader of a GitHub Release. Add `body_path`; the action joins body-then-generated, so the entry leads and the automatic pull-request list follows it. - `CHANGELOG.md` carries the newest toctree entry, which is the version being prepared rather than released: `bump_version.py` produces `1.5.0b4` and `.postN` without adding an entry, so an unconditional body would republish a base version's notes under a vendor refresh that changed no library code. The body is used only when the file's heading names the version being released, and both outcomes are announced in the log. - `util/changelog_md.py --check` was wired into nothing, so drift in a published file was caught only by eye. Add a `changelog` job to `unit-tests.yml`, deliberately ungated by `gate-only` so it also guards the release path that reads the file. Verified: actionlint and shellcheck report the same 15 findings as the base; `--check` exits 0 in step and 1 both on a one-character edit and on a missing file; the selector was exercised against final, pre-release, post-release, dated and absent inputs. Behaviour on a real release is unverified until one runs.
|
✅ GOOD TO MERGE, one judgement call for you: the |
Detailed review (independent, execution-based falsification)Reviewed at base 1. 2. Other upload steps (pypi, conda) can't clobber the body — confirmed by a stronger mechanism than the PR states. Traced 3. Four publishing jobs' tag-gating logic — confirmed undisturbed. 4. Post-release 5. actionlint + shellcheck finding-set claim — independently reproduced, not trusted. Fresh 6. YAML/job-wiring sanity — confirmed. The new 7. Widened blast radius — the judgment call. Both 8. Confirmed no CI has run on this PR at all, consistent with the PR's own "Not verified" section: What remains unverifiable until this actually runs live
VerdictGOOD TO MERGE. No logic errors found across any of the reasoning chains checked. The one judgment call — changelog drift now gating |
JarryShaw
added
the
ci
1 participant
Closes the open question on #521 at
util/changelog_md.py:2— "is it wired in our workflow? like Create Release GHA or some other suitable places?" The answer was no. This wires it in.1. The release body
create-release.yml's release step setgenerate_release_notes: trueand no body at all, so the hand-written changelog never reached a reader of a GitHub Release. It now passesbody_path, andgenerate_release_notesdeliberately stays on.The ordering claim was verified against the pinned version rather than assumed.
softprops/action-gh-release@v3.0.2does not rely on the REST API's own prepend:prepareReleaseMutation(src/github.ts:185-213) fetches the notes itself, forcesgenerate_release_notes = falsebefore the real call, and concatenates client-side as`${releaseParams.body}\n\n${releaseNotes.data.body}`— body first. v3.0.2 is byte-identical to v3.0.3 for that function and forreleaseBody(). Separately, the REST docs for create say the supplied body "will be pre-pended to the automatically generated notes" (api.github.com.json:74377); note the flag is absent from the update schema entirely, which is why the action reimplements it. The generated file is ~17 KB against the action's 124,999-character ceiling, so nothing truncates.2. The post-release decision — it is not only post-releases
The brief asked what happens on a
1.3.5.post44. Working it through, the mismatch is wider than post-releases and live today:CHANGELOG.mdholds the newest toctree entry, i.e. the version being prepared, while the release publishespcapkit.__version__.util/bump_version.pybumps without ever adding an entry — it turns the current1.5.0b3into1.5.0b4, and a final release into.post1. So:CHANGELOG.mdheading1.5.0(final)1.5.01.5.0b4(pre-release)1.5.01.5.0.post1/1.3.5.post441.5.0An unconditional
body_pathwould therefore be wrong on the very next automated release, not merely on some hypothetical post-release. So aSelect release bodystep uses the file only when its heading names the version being released, and otherwise leaves the body unset so the release keeps the generated notes alone — which for a vendor refresh is the honest summary, since the commit list is the only thing that changed. Both outcomes emit a::notice; neither is silent.Only the version token is compared, so an entry still reading
-- unreleasedstill matches. That case gets its own::warninginstead of blocking a release, because the entry is dated by hand at release time and a cosmetic slip should not stop a publish — it should be loud. (Flagging this as a judgement call; drop it if you'd rather it stayed silent.)Leaving the body unset is safe by inspection of the pinned action, not by hope:
body_pathis guarded by a plain truthy check,''is falsy, and the only reader of that input in the wholesrc/tree isreleaseBody()insrc/util.ts:57-68— there is noexistsSyncelsewhere to trip on an empty string.prepareReleaseMutationthen takes itselsebranch and the release still gets the generated notes.The two traps, checked and left alone. The other two
action-gh-releasesteps (pypi, now line 308;conda, now line 459) carry neitherbodynorbody_pathand are untouched — asserted mechanically, see below. The four publishing jobs'startsWith(github.ref_name, 'v') || PCAPKIT_TAG_EXISTS == 'false'gating is byte-identical to the base.3. The drift gate, and an honest correction to #521's description
A
changelogjob inunit-tests.ymlrunsutil/changelog_md.py --check.unit-tests.ymlis the right home because it is the only workflowcreate-release.ymldepends on. Placed there and deliberately ungated bygate-only, the check runs on the release path, so a release body cannot be built from a drifted file — that is the whole point, and it is why this job does not copy theif: inputs.gate-only != truethe matrix jobs use.deploy-pages.ymlalso runs on PRs but cancels superseded runs and is not on the release path, so a gate there would be both weaker and blind to releases. The job needs nopip install: the generator imports onlyargparse,difflib,pathlib,re,sys,typing, verified by running it underpython -S.One correction worth making explicitly. #521's description says the drift gate "is a procedure rather than a mechanism". That is slightly overstated —
tests/project/test_changelog_md.py::RepositoryStateTests::test_committed_changelog_is_in_step_with_the_newest_entryalready asserts the same identity, and already runs in the matrix. So this job's marginal value is narrower than "drift is now gated for the first time", and is threefold: that testskipTests when either file is absent (a silent pass, the exact failure mode to avoid), it reports a mismatch as two 17 KB strings rather than a unified diff, and it does not run early or cheaply.--checkcannot skip, exits 1 on a missing file, and prints a diff.A consequence to be aware of: because the job is ungated, a drifted
CHANGELOG.mdwill now also failcron-vendor.yml's gate and so block the weekly vendor refresh. I think that is correct — the same argument as the existing test gate — but it is a behaviour change, not a no-op.Sequencing
Based on
feat/changelog(#521) at807e376, notmain, so--base feat/changelog.util/changelog_md.pyandCHANGELOG.mddo not exist onmain, so againstmainthe--checkstep would fail on a missing file andbody_pathwould point at nothing. This merges after #521 and should not be merged before it.Basing on the branch is what lets the drift gate be unguarded. The alternative — guarding the step so it no-ops when the file is absent — would have to be loud to be worth anything, and a loud guard for a file that is expected to be missing until #521 lands is just a warning nobody can act on.
.github/is byte-identical betweenmainandfeat/changelog, so this diff is purely additive and will not conflict either way.util/changelog_md.pyis untouched — it belongs to #521.Verified
SC2129/SC2086inGet Version, also present incron-conda.yml).action-gh-releasesteps, exactly 1 setting a body, that one ingithubwithgenerate_release_notes: true; the other two inpypi/condasetting neither; all four gate expressions unchanged; the three pre-existingunit-tests.ymlif:expressions unchanged;changeloghas noif:.--checkexercised locally on this tree: exit 0 in step, exit 1 with a unified diff after a one-character edit, exit 1 whenCHANGELOG.mdis absent, exit 0 again after restore. Runs clean underpython -S.run:block extracted from the YAML and executed against six inputs:1.5.0→path=CHANGELOG.md+ undated warning;1.5.0b4,1.5.0.post1,1.3.5.post44→ no path + notice; dated heading +1.5.0→ path, no warning; absent file → warning, no path, exit 0.Not verified
A workflow change cannot be fully tested without running it. Specifically unverified until a release actually runs: that the rendered release body has the entry above the generated list; that an empty
body_pathbehaves in the live runner as the source says (verified by reading v3.0.2's source, not by executing it); and thechangelogjob's real timing. The::notice/::warningstrings have not been seen rendered in a live log. A greenCreate Releasewith the four publishing jobs skipped is not a release, per that gating — so it would not exercise this path either.This PR runs no GitHub Actions at all, and that is a direct cost of the sequencing. Every
pull_requesttrigger in this repo is restricted tobranches: [main](unit-tests,deploy-pages,python-compatibility,codeql-analysis), so a PR targetingfeat/changelogfires none of them — confirmed,gh run list --branch feat/changelog-ciis empty and the only check is the externalpyup.io/safety-ci, which passed. Thechangelogjob therefore cannot prove itself here; it first executes when #521 reachesmainwith this folded in. Basing onmaininstead would have bought a live CI run at the price of a--checkstep that had to be guarded into uselessness, which seemed the worse trade — but it does mean the local evidence above is the only evidence until #521 lands.