From 19dde53efb704319817b169c0a96d9b4cb94bf42 Mon Sep 17 00:00:00 2001 From: Shane B Date: Sun, 9 Aug 2026 19:58:34 +0200 Subject: [PATCH 1/5] Script the release-notes copy-paste ritual Every release page follows the same convention: only the newest one carries the full wrapper - description, screenshot, feature list, wiki link - and the release it replaces is stripped back to its first line with its changes folded into a collapsed
block. Done by hand that means copying the previous body, swapping the Changes, publishing, then going back to edit the old release. The half that gets forgotten is the demotion, and nobody notices until two releases both look like the current one. The wrapper is not stored in the script. It is read from the release being superseded and carried forward, which is exactly what the copy-paste does - so editing the wording on the latest release changes it for the next one, and the script never needs to know what a release page says. Same reason it reuses the previous heading's wording rather than imposing one: the exporter says "Changes" and the Android app says "What Changed since Last Release", and a tool should not rename either. Versions come from the source - VERSION in the wizard, versionName in the Gradle build - so the tag cannot disagree with what the app reports about itself. Releases are created as drafts, because the release workflow triggers on published: nothing builds until someone clicks the button. Demoting a published release asks first unless --yes. Two things found by running it rather than reading it: - subprocess defaults to cp1252 on Windows and the release bodies contain emoji, so reading a release died in a background thread and gh appeared to return nothing. Encoding is explicit now. - the two kinds of release space their sections differently - one has a blank line after the rule above the changes heading and one does not - so the preamble is preserved verbatim instead of being rebuilt from a guess. 15 tests, including a round trip: putting a release's own changes back through the builder must reproduce it exactly. Anything else means the script is quietly reformatting a page somebody wrote by hand, a little more on every release. Co-Authored-By: Claude Opus 5 --- CONTRIBUTING.md | 23 +- scripts/release_notes.py | 332 +++++++++++++++++++++++++++++ scripts/test/test_release_notes.py | 225 +++++++++++++++++++ 3 files changed, 578 insertions(+), 2 deletions(-) create mode 100644 scripts/release_notes.py create mode 100644 scripts/test/test_release_notes.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 84fcb61a..73cd012e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -449,10 +449,29 @@ So a release is two steps, in this order: git commit -am "Bump the macOS exporter to 1.0.6" git push origin main -# 2. Tag that commit and publish the release -gh release create macos-exporter-v1.0.6 --title "OpenTagViewer MacOS AirTag Exporter 1.0.6" +# 2. Write the changes for this version, then create the release as a draft +python scripts/release_notes.py draft --kind exporter --changes-file notes.md --dry-run +python scripts/release_notes.py draft --kind exporter --changes-file notes.md + +# 3. Publish it from the GitHub UI, then collapse the release it replaced +python scripts/release_notes.py demote --kind exporter ``` +`release_notes.py` exists because the release pages follow a convention that is easy to get +half-right: only the newest release carries the full wrapper — description, screenshot, +feature list, wiki link — and the one it replaces gets stripped back to its first line with +its changes folded into a collapsed `
` block. The half that gets forgotten is the +demotion, and nobody notices until two releases both look current. + +The wrapper is never stored in the script. It is read from the release being superseded and +carried forward, which is what copying the previous body does by hand — so editing the wording +on the latest release is enough to change it for the next one. The version comes from +`VERSION`, and the tag from that, so neither can be typed wrong. `--kind android` does the +same for the app releases. + +The draft is deliberate: the release workflow triggers on `published`, so nothing builds or +ships until someone clicks the button. + The tag must be `macos-exporter-v` followed by exactly what `VERSION` says. Before either build job starts, `test-release-version` runs: diff --git a/scripts/release_notes.py b/scripts/release_notes.py new file mode 100644 index 00000000..0267136c --- /dev/null +++ b/scripts/release_notes.py @@ -0,0 +1,332 @@ +"""Do the release-notes copy-paste ritual, via `gh`. + +Every release page here has the same shape: a wrapper that sells the thing - description, +screenshot, feature list, wiki link - then a "Changes" section for that version, then a footer. +Only the newest release carries the full wrapper. When a new one goes out, the previous release +is demoted: the wrapper is stripped back to its first line and its Changes are folded into a +collapsed `
` block. + +Done by hand that is: copy the previous body, swap the Changes, publish, then go back and edit +the old release. Easy to get half-right, and the half that gets forgotten is the demotion, +which nobody notices until two releases both look like the current one. + +The wrapper is never hardcoded here. It is read from the release being superseded and carried +forward, which is what the copy-paste does anyway - so editing the wording on the latest +release is enough to change it for the next one, and this script does not need to know what a +release page says. + +Usage +----- +See what the new release would say, without creating anything: + + python scripts/release_notes.py draft --kind exporter --changes-file notes.md --dry-run + +Create it as a draft. Nothing builds: the release workflow triggers on `published`, so the +draft sits there until someone clicks the button: + + python scripts/release_notes.py draft --kind exporter --changes-file notes.md + +After publishing, collapse the release it replaced: + + python scripts/release_notes.py demote --kind exporter + +`--kind android` does the same for the Android app releases, which use the same layout with a +differently worded Changes heading. + +The version comes from the source - `VERSION` in the wizard, `versionName` in the Gradle build +- so it cannot disagree with what the app reports about itself. Pass `--version` to override. +""" + +from __future__ import annotations + +import argparse +import json +import re +import subprocess +import sys +import tempfile +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent + +# A heading like "### Changes" or "### What Changed since Last Release". Both kinds of release +# use a heading with "chang" in it, and nothing else in these bodies does. +CHANGES_HEADING = re.compile(r"^#{1,6}\s+.*chang", re.IGNORECASE) + +# A markdown horizontal rule, which is what separates the Changes section from the footer. +HORIZONTAL_RULE = re.compile(r"^\s*(-{3,}|\*{3,}|_{3,})\s*$") + +COLLAPSED_TEMPLATE = """{summary} + +--------------- + +
+Summary + +{changes} + +
+""" + + +class ReleaseError(Exception): + """Something is wrong with the release, the arguments, or gh. The message is the report.""" + + +# --------------------------------------------------------------------------------------- +# Where each kind of release gets its version and its title +# --------------------------------------------------------------------------------------- + +def _exporter_version() -> str: + sys.path.insert(0, str(Path(__file__).resolve().parent)) + import exporter_version + + return exporter_version.read_version()[0] + + +def _android_version() -> str: + gradle = REPO_ROOT / "app" / "build.gradle.kts" + match = re.search(r'^\s*versionName\s*=\s*"([^"]+)"', gradle.read_text(encoding="utf-8"), + re.MULTILINE) + if not match: + raise ReleaseError(f"Could not find versionName in {gradle}") + return match.group(1) + + +KINDS = { + "exporter": { + "prefix": "macos-exporter-v", + "title": "OpenTagViewer MacOS AirTag Exporter v{version}", + "version": _exporter_version, + }, + "android": { + "prefix": "android-app-v", + "title": "OpenTagViewer Android App v{version}", + "version": _android_version, + }, +} + + +# --------------------------------------------------------------------------------------- +# Body parsing - the part worth testing +# --------------------------------------------------------------------------------------- + +def split_body(body: str) -> tuple[str, str, str]: + """ + Split a release body into (preamble, changes, footer). + + `changes` starts at the Changes heading and runs to the next horizontal rule, or to the end + when there is none - the Android releases have no footer. + """ + lines = body.replace("\r\n", "\n").split("\n") + + start = next((i for i, line in enumerate(lines) if CHANGES_HEADING.match(line)), None) + if start is None: + raise ReleaseError( + "Could not find a Changes heading in the release body.\n" + "Expected a markdown heading containing the word 'Changes', for example:\n" + " ### Changes\n" + "If the layout has moved on, this script needs updating rather than working around." + ) + + end = next((i for i in range(start + 1, len(lines)) if HORIZONTAL_RULE.match(lines[i])), + len(lines)) + + # The preamble is kept verbatim, trailing blank lines and all. The two kinds of release + # space their sections differently - one has a blank line after the rule above the changes + # heading and one does not - so reproducing that exactly beats guessing a separator. + return ( + "\n".join(lines[:start]), + "\n".join(lines[start:end]).rstrip(), + "\n".join(lines[end:]).strip(), + ) + + +def build_new_body(previous_body: str, changes: str) -> str: + """Carry the previous release's wrapper forward, with a new Changes section inside it.""" + preamble, previous_changes, footer = split_body(previous_body) + + changes = changes.strip() + if not CHANGES_HEADING.match(changes.split("\n")[0]): + # The caller supplied just the bullet list. Reuse the heading the previous release + # used rather than imposing one: the exporter says "### Changes" and the Android app + # says "### What Changed since Last Release", and neither should be renamed by a tool. + heading = previous_changes.split("\n")[0] + changes = f"{heading}\n\n{changes}" + + # The preamble already carries its own trailing spacing, so a single newline reproduces + # the original layout instead of drifting one blank line further on every release. + body = f"{preamble}\n{changes}" if preamble else changes + if footer: + body = f"{body}\n\n{footer}" + return body + "\n" + + +def demote_body(body: str) -> str: + """ + Rewrite a release body into the collapsed form used by superseded releases. + + Keeps the first line of the wrapper - the one-line description - and folds the Changes into + a `
` block. Everything else in the wrapper goes: the screenshot, the feature list + and the wiki link belong on the current release only. + """ + preamble, changes, _ = split_body(body) + + summary = next((line for line in preamble.split("\n") if line.strip()), "").strip() + if not summary: + raise ReleaseError("The release body has no description line to keep") + + return COLLAPSED_TEMPLATE.format(summary=summary, changes=changes.strip()) + + +def is_already_demoted(body: str) -> bool: + """A body that is already collapsed, so demoting it again would be a no-op.""" + return "Summary" in body + + +# --------------------------------------------------------------------------------------- +# gh +# --------------------------------------------------------------------------------------- + +def _gh(*args: str) -> str: + try: + # encoding is explicit because the default on Windows is cp1252, and these release + # bodies contain emoji - "❓" and "👉" are in the current ones. Without it, reading a + # release fails with a UnicodeDecodeError from a background thread and gh appears to + # have returned nothing at all. + result = subprocess.run(["gh", *args], capture_output=True, text=True, check=False, + encoding="utf-8", cwd=REPO_ROOT) + except FileNotFoundError: + raise ReleaseError( + "The GitHub CLI (gh) is not installed, or not on PATH.\n" + "See CONTRIBUTING.md - it is also what lets an agent read CI failures." + ) + + if result.returncode != 0: + raise ReleaseError(f"gh {' '.join(args)} failed:\n{result.stderr.strip()}") + + return result.stdout + + +def latest_release(prefix: str, include_drafts: bool = False) -> dict: + """The most recent release whose tag starts with `prefix`.""" + raw = _gh("release", "list", "--limit", "100", "--json", + "tagName,name,createdAt,isDraft") + releases = [r for r in json.loads(raw) if r["tagName"].startswith(prefix)] + + if not include_drafts: + releases = [r for r in releases if not r["isDraft"]] + + if not releases: + raise ReleaseError(f"No published release found with a tag starting '{prefix}'") + + return max(releases, key=lambda r: r["createdAt"]) + + +def release_body(tag: str) -> str: + return json.loads(_gh("release", "view", tag, "--json", "body"))["body"] + + +# --------------------------------------------------------------------------------------- +# Commands +# --------------------------------------------------------------------------------------- + +def command_draft(args) -> int: + kind = KINDS[args.kind] + version = args.version or kind["version"]() + tag = f"{kind['prefix']}{version}" + title = kind["title"].format(version=version) + + changes = Path(args.changes_file).read_text(encoding="utf-8") if args.changes_file \ + else args.changes + + previous = latest_release(kind["prefix"]) + body = build_new_body(release_body(previous["tagName"]), changes) + + print(f"Previous release : {previous['tagName']}") + print(f"New release : {tag} ({title})") + print(f"Target : {args.target}") + print() + print(body) + + if args.dry_run: + print("--- dry run, nothing created ---") + return 0 + + with tempfile.NamedTemporaryFile("w", suffix=".md", delete=False, encoding="utf-8") as handle: + handle.write(body) + notes_path = handle.name + + _gh("release", "create", tag, "--draft", "--target", args.target, + "--title", title, "--notes-file", notes_path) + + print(f"Created {tag} as a DRAFT. Nothing is built or published until it is released.") + print(f"After publishing, run: python scripts/release_notes.py demote --kind {args.kind}") + return 0 + + +def command_demote(args) -> int: + kind = KINDS[args.kind] + tag = args.tag or latest_release(kind["prefix"])["tagName"] + + body = release_body(tag) + if is_already_demoted(body): + print(f"{tag} is already collapsed; nothing to do.") + return 0 + + collapsed = demote_body(body) + + print(f"Collapsing {tag} to:\n") + print(collapsed) + + if args.dry_run: + print("--- dry run, nothing changed ---") + return 0 + + if not args.yes: + answer = input(f"Rewrite the notes of the published release {tag}? [y/N] ") + if answer.strip().lower() not in ("y", "yes"): + print("Left alone.") + return 1 + + with tempfile.NamedTemporaryFile("w", suffix=".md", delete=False, encoding="utf-8") as handle: + handle.write(collapsed) + notes_path = handle.name + + _gh("release", "edit", tag, "--notes-file", notes_path) + print(f"{tag} collapsed.") + return 0 + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=(__doc__ or "").split("\n")[0]) + sub = parser.add_subparsers(dest="command", required=True) + + draft = sub.add_parser("draft", help="create the next release as a draft") + draft.add_argument("--kind", choices=sorted(KINDS), required=True) + draft.add_argument("--version", help="override the version read from the source") + draft.add_argument("--target", default="main", help="branch the tag is created from") + group = draft.add_mutually_exclusive_group(required=True) + group.add_argument("--changes-file", help="markdown file holding this version's changes") + group.add_argument("--changes", help="this version's changes, as markdown") + draft.add_argument("--dry-run", action="store_true") + draft.set_defaults(func=command_draft) + + demote = sub.add_parser("demote", help="collapse the release that was just superseded") + demote.add_argument("--kind", choices=sorted(KINDS), required=True) + demote.add_argument("--tag", help="which release to collapse (default: the newest published)") + demote.add_argument("--dry-run", action="store_true") + demote.add_argument("--yes", action="store_true", help="skip the confirmation") + demote.set_defaults(func=command_demote) + + args = parser.parse_args(argv) + + try: + return args.func(args) + except ReleaseError as error: + print(f"\nERROR: {error}\n", file=sys.stderr) + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/test/test_release_notes.py b/scripts/test/test_release_notes.py new file mode 100644 index 00000000..db125ba8 --- /dev/null +++ b/scripts/test/test_release_notes.py @@ -0,0 +1,225 @@ +"""Tests for scripts/release_notes.py. + +The script rewrites the notes of a *published* release, so a parsing mistake is visible to +everyone who visits the releases page. The parsing is also the fragile part: it works by +finding a heading in prose that a human wrote and may reword. + +These cover the pure functions only - nothing here shells out to gh. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import release_notes # noqa: E402 + + +# The real shape of a macOS exporter release: wrapper, changes, footer. +EXPORTER_BODY = """Simple GUI Application for exporting Apple AirTags to a `.zip` file. + +![image](https://example.invalid/screenshot.png) + +The benefit of this GUI Application are: +- Being easily able to see which AirTags are available for export + +❓**How to install/use?** Read wiki page 👉 [here](https://example.invalid/wiki) + +--------------- +### Changes + +- Something that changed in this version +- Something else + +------------- + +An alternative approach is using the export python script from your terminal. +""" + +# The Android releases use a differently worded heading and have no footer. +ANDROID_BODY = """Release version `1.0.4` of the main Android Application + +### Main Features + +- View the current "live" location of your AirTags + +---------------- + +### What Changed since Last Release + +- Added an option to export debug logs +""" + + +# --- splitting -------------------------------------------------------------------------- + +def test_splits_an_exporter_body_into_its_three_parts(): + preamble, changes, footer = release_notes.split_body(EXPORTER_BODY) + + assert preamble.startswith("Simple GUI Application") + assert "screenshot.png" in preamble + assert changes.startswith("### Changes") + assert "Something else" in changes + + # The footer keeps the rule that separates it from the changes, so rebuilding the body + # reproduces the original layout rather than running the two sections together. + assert footer.startswith("---") + assert "An alternative approach" in footer + + +def test_splits_an_android_body_with_no_footer(): + """The heading is worded differently and nothing follows the changes.""" + preamble, changes, footer = release_notes.split_body(ANDROID_BODY) + + assert "Main Features" in preamble + assert changes.startswith("### What Changed since Last Release") + assert "export debug logs" in changes + assert footer == "" + + +def test_the_changes_section_stops_at_the_footer_rule(): + _, changes, _ = release_notes.split_body(EXPORTER_BODY) + + # Otherwise the footer would be carried into the collapsed block on every demotion, + # accumulating a copy of itself each release. + assert "alternative approach" not in changes + + +def test_a_body_with_no_changes_heading_is_an_error(): + with pytest.raises(release_notes.ReleaseError, match="Could not find a Changes heading"): + release_notes.split_body("Just a description, nothing else.") + + +def test_windows_line_endings_are_handled(): + """GitHub stores these bodies with CRLF; a naive split leaves \\r on every line.""" + preamble, changes, _ = release_notes.split_body(EXPORTER_BODY.replace("\n", "\r\n")) + + assert "\r" not in preamble + assert "\r" not in changes + assert changes.startswith("### Changes") + + +# --- building the new release ----------------------------------------------------------- + +def test_carries_the_wrapper_forward_and_swaps_the_changes(): + body = release_notes.build_new_body(EXPORTER_BODY, "- A brand new thing") + + # The wrapper is never hardcoded in the script - it comes from the release being replaced. + assert "Simple GUI Application" in body + assert "screenshot.png" in body + assert "alternative approach" in body + + assert "- A brand new thing" in body + assert "Something that changed in this version" not in body + + +def test_adds_the_changes_heading_when_only_bullets_are_given(): + body = release_notes.build_new_body(EXPORTER_BODY, "- Just a bullet") + + assert "### Changes" in body + + +def test_reuses_the_headings_wording_from_the_previous_release(): + """The Android releases say "What Changed since Last Release"; a tool should not rename it.""" + body = release_notes.build_new_body(ANDROID_BODY, "- Just a bullet") + + assert "### What Changed since Last Release" in body + assert "### Changes" not in body + + +def test_keeps_a_changes_heading_that_was_supplied(): + body = release_notes.build_new_body(EXPORTER_BODY, "### Changes\n\n- Just a bullet") + + assert body.count("### Changes") == 1 + + +@pytest.mark.parametrize("body,changes", [ + (EXPORTER_BODY, "- Something that changed in this version\n- Something else"), + (ANDROID_BODY, "- Added an option to export debug logs"), +]) +def test_rebuilding_with_the_same_changes_reproduces_the_body(body, changes): + """ + The strongest check available without a network: put the changes back and the body should + come out as it went in. Anything else means the script is quietly reformatting a page + somebody wrote by hand, a little more on every release. + """ + assert release_notes.build_new_body(body, changes).strip() == body.strip() + + +def test_builds_a_body_for_a_previous_release_that_had_no_footer(): + body = release_notes.build_new_body(ANDROID_BODY, "- A brand new thing") + + assert "Main Features" in body + assert "- A brand new thing" in body + assert not body.rstrip().endswith("----------------") + + +# --- demoting the superseded release ---------------------------------------------------- + +def test_demoting_keeps_only_the_description_and_collapses_the_changes(): + collapsed = release_notes.demote_body(EXPORTER_BODY) + + assert collapsed.startswith("Simple GUI Application") + assert "Summary" in collapsed + assert "Something that changed in this version" in collapsed + + # The pitch belongs on the current release only. + assert "screenshot.png" not in collapsed + assert "How to install/use" not in collapsed + assert "alternative approach" not in collapsed + + +def test_demoting_is_recognised_as_already_done(): + collapsed = release_notes.demote_body(EXPORTER_BODY) + + # Running it twice would otherwise nest a
inside a
, and the second run + # would keep only the "Summary" line as the description. + assert release_notes.is_already_demoted(collapsed) + assert not release_notes.is_already_demoted(EXPORTER_BODY) + + +def test_demoting_an_android_release_works_too(): + collapsed = release_notes.demote_body(ANDROID_BODY) + + assert collapsed.startswith("Release version `1.0.4`") + assert "What Changed since Last Release" in collapsed + assert "Main Features" not in collapsed + + +def test_demoting_a_body_with_no_description_is_an_error(): + with pytest.raises(release_notes.ReleaseError, match="no description line"): + release_notes.demote_body("### Changes\n\n- Something") + + +# --- versions come from the source ------------------------------------------------------ + +def test_the_exporter_version_is_read_from_the_wizard(): + """ + Not from the tag, and not typed in. The same number is stamped into every export as + `via: OpenTagViewer.app:`, so the release must agree with it. + """ + import exporter_version + + assert release_notes.KINDS["exporter"]["version"]() == exporter_version.read_version()[0] + + +def test_the_android_version_is_read_from_the_gradle_build(): + version = release_notes.KINDS["android"]["version"]() + + assert version + assert version[0].isdigit(), f"{version!r} does not look like a version" + + +def test_the_tag_prefixes_match_what_the_release_workflow_filters_on(): + """ + macos-exporter-python.yml only runs for tags starting 'macos-exporter-v'. A prefix typo + here would create a release that quietly never builds anything. + """ + workflow = (Path(__file__).resolve().parents[2] / ".github" / "workflows" + / "macos-exporter-python.yml").read_text(encoding="utf-8") + + assert release_notes.KINDS["exporter"]["prefix"] in workflow From 5a153a78ea9f5790f12adef56280447b374ba303 Mon Sep 17 00:00:00 2001 From: Shane B Date: Sun, 9 Aug 2026 20:11:29 +0200 Subject: [PATCH 2/5] Collapse the superseded release, not the one just published Demotion defaulted to the newest published release. But it runs *after* the new release goes out, so by then the newest is the one that must keep its wrapper - running it would have stripped the screenshot and feature list off the release published moments earlier. Found by running it: with 1.0.5 already published, `demote --kind exporter` offered to collapse 1.0.5. Now it picks the second newest, the one actually superseded, and refuses when only one release exists rather than guessing. Both tags are printed before the confirmation, so a wrong target is visible before answering rather than after. Co-Authored-By: Claude Opus 5 --- scripts/release_notes.py | 48 ++++++++++++++++++++++++------ scripts/test/test_release_notes.py | 28 +++++++++++++++++ 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/scripts/release_notes.py b/scripts/release_notes.py index 0267136c..defbc4c2 100644 --- a/scripts/release_notes.py +++ b/scripts/release_notes.py @@ -208,19 +208,40 @@ def _gh(*args: str) -> str: return result.stdout -def latest_release(prefix: str, include_drafts: bool = False) -> dict: - """The most recent release whose tag starts with `prefix`.""" +def published_releases(prefix: str) -> list[dict]: + """Published releases whose tag starts with `prefix`, newest first.""" raw = _gh("release", "list", "--limit", "100", "--json", "tagName,name,createdAt,isDraft") - releases = [r for r in json.loads(raw) if r["tagName"].startswith(prefix)] - - if not include_drafts: - releases = [r for r in releases if not r["isDraft"]] + releases = [r for r in json.loads(raw) + if r["tagName"].startswith(prefix) and not r["isDraft"]] if not releases: raise ReleaseError(f"No published release found with a tag starting '{prefix}'") - return max(releases, key=lambda r: r["createdAt"]) + return sorted(releases, key=lambda r: r["createdAt"], reverse=True) + + +def latest_release(prefix: str) -> dict: + """The most recent published release whose tag starts with `prefix`.""" + return published_releases(prefix)[0] + + +def pick_superseded(releases: list[dict]) -> dict: + """ + The release a demotion should collapse: the second newest, not the newest. + + Demotion runs *after* the new release is published, so by then the newest release is the + one that should keep its wrapper. Defaulting to "newest" collapsed the release that had + just gone out - stripping its screenshot and feature list moments after publishing it. + + `releases` must be newest first. + """ + if len(releases) < 2: + raise ReleaseError( + f"Only one published release exists ({releases[0]['tagName']}), so there is " + f"nothing it supersedes. Pass --tag explicitly if you meant to collapse it." + ) + return releases[1] def release_body(tag: str) -> str: @@ -267,7 +288,15 @@ def command_draft(args) -> int: def command_demote(args) -> int: kind = KINDS[args.kind] - tag = args.tag or latest_release(kind["prefix"])["tagName"] + + if args.tag: + tag = args.tag + else: + releases = published_releases(kind["prefix"]) + superseded = pick_superseded(releases) + tag = superseded["tagName"] + print(f"Current release : {releases[0]['tagName']} (keeps its wrapper)") + print(f"Collapsing : {tag}") body = release_body(tag) if is_already_demoted(body): @@ -314,7 +343,8 @@ def main(argv: list[str] | None = None) -> int: demote = sub.add_parser("demote", help="collapse the release that was just superseded") demote.add_argument("--kind", choices=sorted(KINDS), required=True) - demote.add_argument("--tag", help="which release to collapse (default: the newest published)") + demote.add_argument("--tag", help="which release to collapse " + "(default: the one the newest release superseded)") demote.add_argument("--dry-run", action="store_true") demote.add_argument("--yes", action="store_true", help="skip the confirmation") demote.set_defaults(func=command_demote) diff --git a/scripts/test/test_release_notes.py b/scripts/test/test_release_notes.py index db125ba8..7001e79b 100644 --- a/scripts/test/test_release_notes.py +++ b/scripts/test/test_release_notes.py @@ -195,6 +195,34 @@ def test_demoting_a_body_with_no_description_is_an_error(): release_notes.demote_body("### Changes\n\n- Something") +# --- choosing which release to collapse ------------------------------------------------- + +def _release(tag, created): + return {"tagName": tag, "createdAt": created, "isDraft": False} + + +def test_demotion_targets_the_release_that_was_superseded_not_the_newest(): + """ + Demotion runs *after* the new release is published, so by then the newest release is the + one that must keep its wrapper. Defaulting to "newest" collapsed the release that had just + gone out, stripping its screenshot and feature list moments after publishing it. + """ + releases = [ + _release("macos-exporter-v1.0.5", "2026-08-09T18:00:00Z"), + _release("macos-exporter-v1.0.4", "2025-08-15T16:13:20Z"), + _release("macos-exporter-v1.0.3", "2025-07-19T16:42:17Z"), + ] + + assert release_notes.pick_superseded(releases)["tagName"] == "macos-exporter-v1.0.4" + + +def test_a_first_ever_release_has_nothing_to_supersede(): + releases = [_release("macos-exporter-v1.0.0", "2025-03-20T20:34:47Z")] + + with pytest.raises(release_notes.ReleaseError, match="nothing it supersedes"): + release_notes.pick_superseded(releases) + + # --- versions come from the source ------------------------------------------------------ def test_the_exporter_version_is_read_from_the_wizard(): From 18007271463c81c92ebdbfb2ec1aa30a9d0da91c Mon Sep 17 00:00:00 2001 From: Shane B Date: Sun, 9 Aug 2026 20:20:03 +0200 Subject: [PATCH 3/5] Correct what CONTRIBUTING says the tests cover Two claims went stale. The JVM unit tests said "almost nothing here - a green test run says very little". That was accurate this morning and is not now: the stream compositions and decision logic behind the map were extracted out of MapsActivity precisely so they could be tested there, and it is the fastest suite in the project. The scripts/ suite was described as string tooling. It now also covers the release-tag version check and the release-notes script, which is worth naming because each of the three guards something else - and a bug in a guard reports green while protecting nothing. Co-Authored-By: Claude Opus 5 --- CONTRIBUTING.md | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 73cd012e..33c75d2d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -110,7 +110,7 @@ wizard). | Android instrumented tests | `app/src/androidTest/java/` | Gradle / JUnit + emulator | provisioned for you | | Chaquopy bridge tests | `app/src/test/python/` | pytest | no | | Desktop wizard tests | `python/test/` | pytest | no | -| String tooling tests | `scripts/test/` | pytest | no | +| Tooling tests | `scripts/test/` | pytest | no | ### Run everything @@ -172,8 +172,15 @@ Targeting a subset: ### Android unit tests -Plain JVM, no Android framework. Be aware there is currently almost nothing here — the -meaningful coverage is instrumented, so a green `test` run says very little. +Plain JVM, no Android framework, so these are the fastest tests in the project — seconds, no +emulator. + +Most of what lives here is in `util/rx/`: the stream compositions and decision logic behind +the map, extracted out of `MapsActivity` precisely so it could be tested. They assert that a +call *happens* rather than that a value looks right, because the failures in this area are +silent — a stream disposed early, a marker that stops being raised, a fetch that returns +nothing being indistinguishable from a fetch that failed. None of those throw, and none show +up in logcat. ```bash ./gradlew testDebugUnitTest @@ -214,14 +221,21 @@ python -m pytest ./test flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics ``` -### String tooling tests +### Tooling tests ```bash python -m pytest scripts/test -v ``` -`scripts/add_strings.py` gates CI's translation check, so a bug in it would report green -while protecting nothing. +Most of `scripts/` is one-shot utilities where a failure is loud and immediate to whoever ran +them, and tests would not earn their keep. These three are different, because each one guards +something else and a bug in a guard reports green while protecting nothing: + +| Script | What it gates | +| --- | --- | +| `add_strings.py` | CI's translation check | +| `exporter_version.py` | whether a release tag may disagree with the version in the source | +| `release_notes.py` | the notes of a *published* release, so a parsing mistake is visible to everyone | ### Which Python each tree targets From a0910daf119fb9a56c12b9be1172b4aec3a25511 Mon Sep 17 00:00:00 2001 From: Shane B Date: Sun, 9 Aug 2026 20:33:46 +0200 Subject: [PATCH 4/5] Refuse to draft a release for a version that already exists Running `draft --kind android` today computed android-app-v1.0.4 - the tag already released - because versionName in the Gradle build has not been bumped. The script happily offered to create a draft on top of a published release, with notes describing changes that release does not contain. Reading the version from the source is what makes the tag trustworthy, and it is also what makes forgetting to bump it produce a plausible-looking wrong answer rather than an error. Now it stops, and says which file to bump for that kind of release. Drafts count as taken too: a half-prepared release still occupies the tag. Co-Authored-By: Claude Opus 5 --- scripts/release_notes.py | 27 +++++++++++++++++++++++++++ scripts/test/test_release_notes.py | 26 ++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/scripts/release_notes.py b/scripts/release_notes.py index defbc4c2..f57f6fa6 100644 --- a/scripts/release_notes.py +++ b/scripts/release_notes.py @@ -98,11 +98,13 @@ def _android_version() -> str: "prefix": "macos-exporter-v", "title": "OpenTagViewer MacOS AirTag Exporter v{version}", "version": _exporter_version, + "bump": 'python/main/wizard.py -> VERSION = "..."', }, "android": { "prefix": "android-app-v", "title": "OpenTagViewer Android App v{version}", "version": _android_version, + "bump": 'app/build.gradle.kts -> versionName = "..." (and versionCode)', }, } @@ -226,6 +228,27 @@ def latest_release(prefix: str) -> dict: return published_releases(prefix)[0] +def check_version_is_new(tag: str, existing_tags: list[str], bump_hint: str) -> None: + """ + Refuse to release a version that already exists. + + The version is read from the source, so if nobody bumped it the computed tag is the one + already released - and the script would otherwise offer to create a draft on top of a + published release, with notes describing changes that release does not contain. + """ + if tag in existing_tags: + raise ReleaseError( + f"{tag} has already been released.\n" + f"\n" + f"The version comes from the source, so this means it has not been bumped yet:\n" + f"\n" + f" {bump_hint}\n" + f"\n" + f"Bump it, commit it, and run this again. Releasing is two steps on purpose - the\n" + f"bump is a commit, and the tag only publishes it." + ) + + def pick_superseded(releases: list[dict]) -> dict: """ The release a demotion should collapse: the second newest, not the newest. @@ -261,6 +284,10 @@ def command_draft(args) -> int: changes = Path(args.changes_file).read_text(encoding="utf-8") if args.changes_file \ else args.changes + # Drafts count: a half-prepared release still occupies the tag. + raw = _gh("release", "list", "--limit", "100", "--json", "tagName") + check_version_is_new(tag, [r["tagName"] for r in json.loads(raw)], kind["bump"]) + previous = latest_release(kind["prefix"]) body = build_new_body(release_body(previous["tagName"]), changes) diff --git a/scripts/test/test_release_notes.py b/scripts/test/test_release_notes.py index 7001e79b..18fea639 100644 --- a/scripts/test/test_release_notes.py +++ b/scripts/test/test_release_notes.py @@ -195,6 +195,32 @@ def test_demoting_a_body_with_no_description_is_an_error(): release_notes.demote_body("### Changes\n\n- Something") +# --- refusing to release a version that already exists ---------------------------------- + +def test_releasing_an_unbumped_version_is_refused(): + """ + The version is read from the source, so forgetting to bump it computes the tag that is + already released - and without this the script would offer to draft a release on top of a + published one, with notes describing changes it does not contain. + """ + with pytest.raises(release_notes.ReleaseError, match="has already been released"): + release_notes.check_version_is_new( + "android-app-v1.0.4", + ["android-app-v1.0.4", "android-app-v1.0.3"], + 'app/build.gradle.kts -> versionName') + + +def test_the_refusal_says_where_to_bump_the_version(): + with pytest.raises(release_notes.ReleaseError, match="app/build.gradle.kts"): + release_notes.check_version_is_new( + "android-app-v1.0.4", ["android-app-v1.0.4"], 'app/build.gradle.kts -> versionName') + + +def test_a_bumped_version_is_allowed(): + release_notes.check_version_is_new( + "android-app-v1.0.5", ["android-app-v1.0.4", "android-app-v1.0.3"], "hint") + + # --- choosing which release to collapse ------------------------------------------------- def _release(tag, created): From fd5d0d9eff8c853d9792cc6517e109db9dfa5e40 Mon Sep 17 00:00:00 2001 From: Shane B Date: Sun, 9 Aug 2026 22:06:19 +0200 Subject: [PATCH 5/5] Read versions through release_version rather than re-implementing them Rebases onto the rename, and removes the duplication the rename exposed: release_notes.py had its own copy of where each version lives, including a second regex for versionName in the Gradle build. Two readers of the same fact can drift, and the failure here would be a release page describing one version while the build inside it reports another - the exact thing release_version.py exists to prevent. It also now owns both tag prefixes, so there is one place that decides what a release is called. What is left in this script's KINDS is only what a release *page* needs: the title, and which file to tell someone to bump. Tests assert the two modules agree rather than duplicating the knowledge again: that the versions come from release_version's readers, that both tag prefixes match what their workflow filters on - build-release.yml included, which was never covered - and that neither module knows a kind the other does not. Co-Authored-By: Claude Opus 5 --- scripts/release_notes.py | 47 +++++++++++++++--------------- scripts/test/test_release_notes.py | 40 ++++++++++++++++--------- 2 files changed, 50 insertions(+), 37 deletions(-) diff --git a/scripts/release_notes.py b/scripts/release_notes.py index f57f6fa6..ac951e2a 100644 --- a/scripts/release_notes.py +++ b/scripts/release_notes.py @@ -47,6 +47,10 @@ import tempfile from pathlib import Path +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +import release_version # noqa: E402 + REPO_ROOT = Path(__file__).resolve().parent.parent # A heading like "### Changes" or "### What Changed since Last Release". Both kinds of release @@ -77,38 +81,37 @@ class ReleaseError(Exception): # Where each kind of release gets its version and its title # --------------------------------------------------------------------------------------- -def _exporter_version() -> str: - sys.path.insert(0, str(Path(__file__).resolve().parent)) - import exporter_version - - return exporter_version.read_version()[0] - +def _version(kind: str) -> str: + """ + The version the source declares, read by the same module CI checks the tag against. -def _android_version() -> str: - gradle = REPO_ROOT / "app" / "build.gradle.kts" - match = re.search(r'^\s*versionName\s*=\s*"([^"]+)"', gradle.read_text(encoding="utf-8"), - re.MULTILINE) - if not match: - raise ReleaseError(f"Could not find versionName in {gradle}") - return match.group(1) + Delegated rather than re-implemented: release_version.py already knows where each version + lives and how to read it, and it is what fails a release whose tag disagrees. If this + script read the version its own way, the two could differ - and the failure would be a + release page describing one version while the build inside it reports another, which is + the exact problem release_version.py exists to prevent. + """ + return release_version.KINDS[kind]["read"]()[0] +# Only what a release *page* needs. The version and the tag prefix come from +# release_version.py, so there is one place that decides what a release is called. KINDS = { "exporter": { - "prefix": "macos-exporter-v", "title": "OpenTagViewer MacOS AirTag Exporter v{version}", - "version": _exporter_version, "bump": 'python/main/wizard.py -> VERSION = "..."', }, "android": { - "prefix": "android-app-v", "title": "OpenTagViewer Android App v{version}", - "version": _android_version, "bump": 'app/build.gradle.kts -> versionName = "..." (and versionCode)', }, } +def _prefix(kind: str) -> str: + return release_version.KINDS[kind]["prefix"] + + # --------------------------------------------------------------------------------------- # Body parsing - the part worth testing # --------------------------------------------------------------------------------------- @@ -277,8 +280,8 @@ def release_body(tag: str) -> str: def command_draft(args) -> int: kind = KINDS[args.kind] - version = args.version or kind["version"]() - tag = f"{kind['prefix']}{version}" + version = args.version or _version(args.kind) + tag = f"{_prefix(args.kind)}{version}" title = kind["title"].format(version=version) changes = Path(args.changes_file).read_text(encoding="utf-8") if args.changes_file \ @@ -288,7 +291,7 @@ def command_draft(args) -> int: raw = _gh("release", "list", "--limit", "100", "--json", "tagName") check_version_is_new(tag, [r["tagName"] for r in json.loads(raw)], kind["bump"]) - previous = latest_release(kind["prefix"]) + previous = latest_release(_prefix(args.kind)) body = build_new_body(release_body(previous["tagName"]), changes) print(f"Previous release : {previous['tagName']}") @@ -314,12 +317,10 @@ def command_draft(args) -> int: def command_demote(args) -> int: - kind = KINDS[args.kind] - if args.tag: tag = args.tag else: - releases = published_releases(kind["prefix"]) + releases = published_releases(_prefix(args.kind)) superseded = pick_superseded(releases) tag = superseded["tagName"] print(f"Current release : {releases[0]['tagName']} (keeps its wrapper)") diff --git a/scripts/test/test_release_notes.py b/scripts/test/test_release_notes.py index 18fea639..05e6ddaa 100644 --- a/scripts/test/test_release_notes.py +++ b/scripts/test/test_release_notes.py @@ -251,29 +251,41 @@ def test_a_first_ever_release_has_nothing_to_supersede(): # --- versions come from the source ------------------------------------------------------ -def test_the_exporter_version_is_read_from_the_wizard(): +def test_the_exporter_version_comes_from_the_module_that_gates_the_release(): """ - Not from the tag, and not typed in. The same number is stamped into every export as - `via: OpenTagViewer.app:`, so the release must agree with it. + Not from the tag, and not typed in - and specifically read by release_version.py, the same + module CI uses to fail a release whose tag disagrees. Two readers could drift apart, and + the result would be a release page describing one version while the build inside it + reports another. """ - import exporter_version + import release_version - assert release_notes.KINDS["exporter"]["version"]() == exporter_version.read_version()[0] + assert release_notes._version("exporter") == release_version.read_version()[0] -def test_the_android_version_is_read_from_the_gradle_build(): - version = release_notes.KINDS["android"]["version"]() +def test_the_android_version_comes_from_the_same_place_too(): + import release_version - assert version - assert version[0].isdigit(), f"{version!r} does not look like a version" + assert release_notes._version("android") == release_version.read_gradle_version()[0] -def test_the_tag_prefixes_match_what_the_release_workflow_filters_on(): +@pytest.mark.parametrize("kind,workflow_name", [ + ("exporter", "macos-exporter-python.yml"), + ("android", "build-release.yml"), +]) +def test_the_tag_prefixes_match_what_the_release_workflows_filter_on(kind, workflow_name): """ - macos-exporter-python.yml only runs for tags starting 'macos-exporter-v'. A prefix typo - here would create a release that quietly never builds anything. + Each release workflow only runs for tags starting with its prefix. A prefix that did not + match would create a release that quietly never builds anything. """ workflow = (Path(__file__).resolve().parents[2] / ".github" / "workflows" - / "macos-exporter-python.yml").read_text(encoding="utf-8") + / workflow_name).read_text(encoding="utf-8") + + assert release_notes._prefix(kind) in workflow + + +def test_every_kind_this_script_knows_is_one_release_version_knows(): + """Otherwise a kind here would compute a tag with no prefix and no version to read.""" + import release_version - assert release_notes.KINDS["exporter"]["prefix"] in workflow + assert set(release_notes.KINDS) == set(release_version.KINDS)