ci: standardize release process#8
Conversation
Add scripts/release.sh for version bumps, changelog updates, tagging, and GitHub Release creation via CI. Enforce changelog checks on version PRs.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| unreleased = re.search(r"^## \[Unreleased\]\s*\n(.*?)(?=^## \[|\Z)", text, re.M | re.S) | ||
| if unreleased and unreleased.group(1).strip(): | ||
| body = unreleased.group(1).rstrip() + "\n" | ||
| text = re.sub(r"^## \[Unreleased\]\s*\n.*?(?=^## \[|\Z)", "", text, count=1, flags=re.M | re.S) | ||
| insert = f"## [Unreleased]\n\n## [{ver}] - {date}\n{body}\n" | ||
| if text.startswith("# Changelog"): | ||
| parts = text.split("\n\n", 2) | ||
| if len(parts) >= 2: | ||
| text = parts[0] + "\n\n" + parts[1] + "\n\n" + insert + (parts[2] if len(parts) > 2 else "") | ||
| else: | ||
| text = insert + text | ||
| else: | ||
| text = insert + text | ||
| else: | ||
| insert = ( | ||
| f"## [Unreleased]\n\n" | ||
| f"## [{ver}] - {date}\n\n" | ||
| "### Changed\n\n" | ||
| f"- Release {ver}\n\n" | ||
| ) | ||
| if "## [Unreleased]" in text: | ||
| text = text.replace("## [Unreleased]", insert.strip() + "\n\n## [Unreleased]", 1) | ||
| else: | ||
| text = re.sub(r"(^# Changelog.*?)(\n\n|\Z)", r"\1\n\n" + insert, text, count=1, flags=re.S) |
There was a problem hiding this comment.
Blocking — update_changelog corrupts the actual CHANGELOG.md format used in this PR.
Tracing both branches against the CHANGELOG.md added in this same PR (which has the standard Keep a Changelog preamble + an empty ## [Unreleased]):
Branch 1 — empty ## [Unreleased] (common: someone forgets to add notes before running prepare):
unreleased.group(1).strip()is empty → falls through to theelseblock.insert.strip()already begins with## [Unreleased], thentext.replace("## [Unreleased]", insert.strip() + "\n\n## [Unreleased]", 1)substitutes the original heading with the new content followed by another## [Unreleased]heading.- Result: the changelog now has two
## [Unreleased]sections, which then tripscheck-release.pyon the next release (and any tooling that assumes a single Unreleased section).
Branch 2 — ## [Unreleased] has content:
- After removing the Unreleased section,
text.startswith("# Changelog")is true, sotext.split("\n\n", 2)produces three parts. Because the current preamble has two blank-line breaks before the first version section,parts[1]is just"All notable changes…"andparts[2]starts with"The format is based on…\n\n## [0.1.1]…". - The new section gets inserted between
parts[1]andparts[2], which shoves the "The format is based on…" paragraph after the new version section and between the new version and the older versions. - Result: the preamble ends up interleaved with version history.
Both bugs hit the very file shipped here. Suggested simpler replacement that operates in place on the Unreleased section without splitting on \n\n:
def update_changelog(...):
...
# Replace `## [Unreleased]\n\n<body>` with
# `## [Unreleased]\n\n## [ver] - date\n\n<body>` in a single substitution.
pattern = r"(^## \[Unreleased\]\s*\n)(.*?)(?=^## \[|\Z)"
def repl(m):
body = m.group(2).strip()
block = f"## [{ver}] - {date}\n\n"
if body:
block += body + "\n\n"
else:
block += f"### Changed\n\n- Release {ver}\n\n"
return m.group(1) + "\n" + block
new_text, n = re.subn(pattern, repl, text, count=1, flags=re.M | re.S)
if n == 0:
raise SystemExit("CHANGELOG.md is missing a '## [Unreleased]' section")
path.write_text(new_text)Either way, please add a unit test (or at least an examples/-style fixture run) so this regresses loudly if anyone touches it — the current logic is too easy to break silently.
| { | ||
| echo "body<<EOF" | ||
| echo "$body" | ||
| echo "EOF" | ||
| } >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
nit: using EOF as the heredoc delimiter for multiline GITHUB_OUTPUT is fragile — if any future changelog entry contains a line that is exactly EOF, this step will produce a malformed output and the release notes will be wrong or the step will fail. GitHub's own docs recommend a random delimiter. (not blocking)
delim="EOF_$(openssl rand -hex 8)"
{
echo "body<<$delim"
echo "$body"
echo "$delim"
} >> "$GITHUB_OUTPUT"
| name: ${{ steps.meta.outputs.name }} ${{ steps.meta.outputs.version }} | ||
| body: ${{ steps.notes.outputs.body }} | ||
| generate_release_notes: false | ||
| make_latest: true |
There was a problem hiding this comment.
nit: make_latest: true is unconditional, but the tag pattern 'v[0-9]*' matches pre-releases like v1.0.0a1 / v2.0.0rc1. Marking a pre-release as "latest" on GitHub is usually wrong (it surfaces as the default download for users). Consider deriving this from whether the version has a PEP 440 suffix, e.g. compute is_prerelease in the meta step and pass make_latest: ${{ steps.meta.outputs.is_prerelease == 'false' }}. (not blocking)
| base = "origin/main" | ||
| for candidate in ("origin/main", "origin/master"): | ||
| if subprocess.call(["git", "rev-parse", "--verify", candidate], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) == 0: | ||
| base = candidate | ||
| break |
There was a problem hiding this comment.
nit: this hardcodes the base to origin/main (or origin/master) instead of using the PR's actual base ref. For a PR that targets a non-default branch (release branch, stacked PR), the diff is taken against the wrong baseline and could either skip the check incorrectly or fail spuriously. In pull_request events the base is available as $GITHUB_BASE_REF:
base_ref = os.environ.get("GITHUB_BASE_REF") or "main"
base = f"origin/{base_ref}"(not blocking — fine for the standard PR-to-main flow this repo uses today.)
| cmd_publish() { | ||
| need gh | ||
| need python3 | ||
| ensure_clean | ||
|
|
||
| local base ver tag | ||
| base="$(default_branch)" | ||
| git fetch origin "$base" | ||
| git checkout "$base" | ||
| git pull --ff-only origin "$base" | ||
| ensure_clean | ||
|
|
||
| ver="$(get_version)" | ||
| tag="v${ver}" | ||
|
|
||
| git rev-parse "$tag" >/dev/null 2>&1 && die "tag $tag already exists" |
There was a problem hiding this comment.
nit: cmd_publish only fetches the base branch, not tags, so git rev-parse "$tag" won't see a tag that already exists on the remote but not locally. Two engineers running publish against the same version will both pass this check; the second one will then fail at git push origin "$tag" with a less clear error. Adding git fetch --tags origin before the check would surface the conflict with the script's own die message. (not blocking)
| next_heading = re.search(r"^## \[", rest, re.M) | ||
| end = match.end() + (next_heading.start() if next_heading else len(rest)) | ||
| section = changelog[start:end].strip() | ||
| title, _, body = section.partition("\n") |
There was a problem hiding this comment.
super nit: title is assigned but never used — _, _, body = section.partition("\n") or just slicing past the first newline reads a bit cleaner. (not blocking)
There was a problem hiding this comment.
Review
Blocking Issues
scripts/release.shupdate_changelog(lines 122–145) corruptsCHANGELOG.mdin both branches when run against the format used in this PR. The empty-Unreleasedpath produces a duplicate## [Unreleased]heading, and the non-empty path moves the preamble paragraph below the new version section. See inline comment for the traced output and a suggested rewrite.
Action Required
- Rewrite
update_changelogso that it edits the Unreleased section in place (suggestion provided inline) and add a small test/fixture run so regressions are caught.
Non-blocking nits left inline on release.yml, check-release.py, release.sh (cmd_publish), and extract-changelog.py.
| unreleased = re.search(r"^## \[Unreleased\]\s*\n(.*?)(?=^## \[|\Z)", text, re.M | re.S) | ||
| if unreleased and unreleased.group(1).strip(): | ||
| body = unreleased.group(1).rstrip() + "\n" | ||
| text = re.sub(r"^## \[Unreleased\]\s*\n.*?(?=^## \[|\Z)", "", text, count=1, flags=re.M | re.S) | ||
| insert = f"## [Unreleased]\n\n## [{ver}] - {date}\n{body}\n" | ||
| if text.startswith("# Changelog"): | ||
| parts = text.split("\n\n", 2) | ||
| if len(parts) >= 2: | ||
| text = parts[0] + "\n\n" + parts[1] + "\n\n" + insert + (parts[2] if len(parts) > 2 else "") | ||
| else: | ||
| text = insert + text | ||
| else: | ||
| text = insert + text | ||
| else: | ||
| insert = ( | ||
| f"## [Unreleased]\n\n" | ||
| f"## [{ver}] - {date}\n\n" | ||
| "### Changed\n\n" | ||
| f"- Release {ver}\n\n" | ||
| ) | ||
| if "## [Unreleased]" in text: | ||
| text = text.replace("## [Unreleased]", insert.strip() + "\n\n## [Unreleased]", 1) | ||
| else: | ||
| text = re.sub(r"(^# Changelog.*?)(\n\n|\Z)", r"\1\n\n" + insert, text, count=1, flags=re.S) |
There was a problem hiding this comment.
Blocking — still unfixed from review #1. The update_changelog logic is unchanged since cycle 1, and both branches still corrupt the CHANGELOG.md shipped in this PR:
- Empty-
Unreleasedpath (line 143) produces two## [Unreleased]headings after the first run, which then tripscheck-release.pyand any other tooling assuming a single Unreleased section. - Non-empty-
Unreleasedpath (line 128–130) splits the preamble on\n\nand inserts betweenparts[1]andparts[2], which shoves the "The format is based on…" paragraph below the new version section.
Please replace this with an in-place substitution on the Unreleased block (suggestion from the prior thread reproduced below) and add at least one fixture/unit test so this regresses loudly next time it's touched.
pattern = r"(^## \[Unreleased\]\s*\n)(.*?)(?=^## \[|\Z)"
def repl(m):
body = m.group(2).strip()
block = f"## [{ver}] - {date}\n\n"
if body:
block += body + "\n\n"
else:
block += f"### Changed\n\n- Release {ver}\n\n"
return m.group(1) + "\n" + block
new_text, n = re.subn(pattern, repl, text, count=1, flags=re.M | re.S)
if n == 0:
raise SystemExit("CHANGELOG.md is missing a '## [Unreleased]' section")
path.write_text(new_text)
There was a problem hiding this comment.
Review
Blocking Issues
scripts/release.shupdate_changelog(lines 122–145) is unchanged from cycle 1 and still corrupts theCHANGELOG.mdshipped in this PR — see the inline thread for the two failure modes and a suggested rewrite.
Action Required
- Rewrite
update_changelogto edit the[Unreleased]block in place (suggestion provided inline). - Add a fixture or unit test so this regresses loudly.
Non-blocking nits from cycle 1 (release.yml heredoc delimiter, make_latest for pre-releases, check-release.py base ref, cmd_publish tag fetch, extract-changelog.py unused title) were not re-raised — leaving those to the author's discretion.
Avoid duplicate [Unreleased] headings and keep the preamble intact when preparing releases. Add unit tests and harden release workflow output.
There was a problem hiding this comment.
Prior blocking issue resolved: update_changelog logic was extracted to scripts/update_changelog.py with an in-place substitution and tests/test_update_changelog.py exercises both the empty-Unreleased and populated-Unreleased paths, including the exact assertions that would have caught the cycle-1/2 regressions (single ## [Unreleased] heading; preamble stays above the new version). Traced against the in-PR CHANGELOG.md — output is well-formed in both branches.
1 participant
Summary
scripts/release.sh(prepare + publish)release.yml)check-release.yml)RELEASING.mdandCHANGELOG.mdTest plan
./scripts/release.sh prepare patchon next release (post-merge)