mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-03 09:48:06 +00:00
fix(version): address review — regex strictness, OSError, stale doc claim
From CodeRabbit on #428, all three valid: - The module docstring claimed the tag check "runs at release time in .github/workflows/release-version-check.yml". That workflow is held back to a follow-up PR (the pushing token lacks the `workflow` scope), so the claim was false as written. Both files now describe the script as a manual pre-flight and say the CI wiring is still to come. - `\d` also matches non-ASCII decimal digits, which int() happily parses, and `\s` matches newlines -- so "##\n3.2.0" read as a version heading. Patterns now use [0-9] and [ \t], kept in step across the test and the script, with a regression test pinning both behaviours. - A missing or unreadable CHANGELOG.md raised OSError out of read_text() and printed a traceback. In a release gate that reads as "the tooling is broken"; it now reports the path and a recovery action and exits 1. Verified: v3.2.0 passes, a mismatched tag exits 1, and a missing CHANGELOG exits 1 with the new message instead of a traceback. 5 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
This commit is contained in:
co-authored by
Claude Opus 5
parent
4f28d4eb64
commit
e26ed29385
@@ -5,9 +5,9 @@ Run it *before* creating a tag to check yourself:
|
|||||||
|
|
||||||
python scripts/check_release_version.py v3.2.0
|
python scripts/check_release_version.py v3.2.0
|
||||||
|
|
||||||
CI runs it on every pushed `v*` tag and published release
|
Wiring it into CI (on pushed `v*` tags and published releases) is a follow-up
|
||||||
(`.github/workflows/release-version-check.yml`), so a mismatch shows up as a
|
PR, so for now it is a manual pre-flight: run it before creating the tag and a
|
||||||
red check on the release rather than as a silent wrong answer on user devices.
|
mismatch shows up here rather than as a silent wrong answer on user devices.
|
||||||
|
|
||||||
Why this exists: `v3.1.0` was tagged 2026-05-31 while `src/__init__.py` still
|
Why this exists: `v3.1.0` was tagged 2026-05-31 while `src/__init__.py` still
|
||||||
said `"1.0.0"`; the bump to `"3.1.0"` did not land until 2026-07-12. Devices
|
said `"1.0.0"`; the bump to `"3.1.0"` did not land until 2026-07-12. Devices
|
||||||
@@ -27,8 +27,13 @@ from pathlib import Path
|
|||||||
REPO_ROOT = Path(__file__).resolve().parents[1]
|
REPO_ROOT = Path(__file__).resolve().parents[1]
|
||||||
sys.path.insert(0, str(REPO_ROOT))
|
sys.path.insert(0, str(REPO_ROOT))
|
||||||
|
|
||||||
SEMVER = re.compile(r"^\d+\.\d+\.\d+$")
|
# [0-9] rather than \d, and [ \t] rather than \s: \d also matches non-ASCII
|
||||||
HEADING = re.compile(r"^##\s+(?P<version>\d+\.\d+\.\d+)\s*$", re.MULTILINE)
|
# decimal digits (which int() parses), and \s matches newlines, so "##\n3.2.0"
|
||||||
|
# would otherwise read as a version heading. Keep these in step with
|
||||||
|
# test/test_version_consistency.py.
|
||||||
|
SEMVER = re.compile(r"^[0-9]+\.[0-9]+\.[0-9]+$")
|
||||||
|
HEADING = re.compile(
|
||||||
|
r"^##[ \t]+(?P<version>[0-9]+\.[0-9]+\.[0-9]+)[ \t]*$", re.MULTILINE)
|
||||||
|
|
||||||
|
|
||||||
def normalize(tag: str) -> str:
|
def normalize(tag: str) -> str:
|
||||||
@@ -37,6 +42,13 @@ def normalize(tag: str) -> str:
|
|||||||
|
|
||||||
|
|
||||||
def newest_changelog_version(changelog: Path) -> str | None:
|
def newest_changelog_version(changelog: Path) -> str | None:
|
||||||
|
"""Newest version heading, or None when there is none.
|
||||||
|
|
||||||
|
Raises OSError if the file cannot be read; main() turns that into a clear
|
||||||
|
message rather than a traceback, because this runs as a release gate and a
|
||||||
|
traceback there reads as "the tooling is broken", not "your CHANGELOG is
|
||||||
|
missing".
|
||||||
|
"""
|
||||||
headings = HEADING.findall(changelog.read_text(encoding="utf-8"))
|
headings = HEADING.findall(changelog.read_text(encoding="utf-8"))
|
||||||
return headings[0] if headings else None
|
return headings[0] if headings else None
|
||||||
|
|
||||||
@@ -52,10 +64,21 @@ def main() -> int:
|
|||||||
from src import __version__ as core_version
|
from src import __version__ as core_version
|
||||||
|
|
||||||
tag_version = normalize(args.tag)
|
tag_version = normalize(args.tag)
|
||||||
changelog_version = newest_changelog_version(REPO_ROOT / "CHANGELOG.md")
|
changelog_path = REPO_ROOT / "CHANGELOG.md"
|
||||||
|
|
||||||
problems: list[str] = []
|
problems: list[str] = []
|
||||||
|
|
||||||
|
try:
|
||||||
|
changelog_version = newest_changelog_version(changelog_path)
|
||||||
|
except OSError as e:
|
||||||
|
print(
|
||||||
|
f"Release version check FAILED for tag {args.tag}:\n"
|
||||||
|
f" - could not read {changelog_path}: {e}\n"
|
||||||
|
f" Restore the file (git checkout -- CHANGELOG.md) and re-run.",
|
||||||
|
file=sys.stderr,
|
||||||
|
)
|
||||||
|
return 1
|
||||||
|
|
||||||
if not SEMVER.match(tag_version):
|
if not SEMVER.match(tag_version):
|
||||||
problems.append(
|
problems.append(
|
||||||
f"tag {args.tag!r} is not vX.Y.Z. Older tags (v2.5) predate this "
|
f"tag {args.tag!r} is not vX.Y.Z. Older tags (v2.5) predate this "
|
||||||
|
|||||||
@@ -13,8 +13,12 @@ which is below the `(2, 0, 0)` floor in `PluginLoader._warn_if_incompatible` —
|
|||||||
so those users get no compatibility warning at all. See
|
so those users get no compatibility warning at all. See
|
||||||
`docs/SPORTS_UNIFICATION.md` (phase B4).
|
`docs/SPORTS_UNIFICATION.md` (phase B4).
|
||||||
|
|
||||||
The matching tag check runs at release time in
|
A tag is not available here, so the tag half of the check lives in
|
||||||
`.github/workflows/release-version-check.yml`; a tag is not available here.
|
`scripts/check_release_version.py`. Wiring that script into CI (on pushed `v*`
|
||||||
|
tags and published releases) is a follow-up PR; until it lands, run it by hand
|
||||||
|
before tagging:
|
||||||
|
|
||||||
|
python scripts/check_release_version.py v3.2.0
|
||||||
|
|
||||||
Note: `src.plugin_system.__version__` is deliberately NOT checked. That module
|
Note: `src.plugin_system.__version__` is deliberately NOT checked. That module
|
||||||
versions the *plugin API* (it sits beside `__api_version__` and is documented as
|
versions the *plugin API* (it sits beside `__api_version__` and is documented as
|
||||||
@@ -31,10 +35,15 @@ import src
|
|||||||
REPO_ROOT = Path(__file__).resolve().parents[1]
|
REPO_ROOT = Path(__file__).resolve().parents[1]
|
||||||
CHANGELOG = REPO_ROOT / "CHANGELOG.md"
|
CHANGELOG = REPO_ROOT / "CHANGELOG.md"
|
||||||
|
|
||||||
SEMVER = re.compile(r"^(\d+)\.(\d+)\.(\d+)$")
|
# [0-9] rather than \d: \d also matches non-ASCII decimal digits, which int()
|
||||||
|
# happily parses, so a heading in Arabic-Indic numerals would pass the pattern
|
||||||
|
# and then mismatch confusingly. [ \t] rather than \s for the same class of
|
||||||
|
# reason -- \s matches newlines, so "##\n3.2.0" would read as a heading.
|
||||||
|
SEMVER = re.compile(r"^([0-9]+)\.([0-9]+)\.([0-9]+)$")
|
||||||
# Version headings look like "## 3.2.0". A leading "## Unreleased" section is
|
# Version headings look like "## 3.2.0". A leading "## Unreleased" section is
|
||||||
# allowed and skipped -- it is where module additions are staged before a bump.
|
# allowed and skipped -- it is where module additions are staged before a bump.
|
||||||
HEADING = re.compile(r"^##\s+(?P<version>\d+\.\d+\.\d+)\s*$", re.MULTILINE)
|
HEADING = re.compile(
|
||||||
|
r"^##[ \t]+(?P<version>[0-9]+\.[0-9]+\.[0-9]+)[ \t]*$", re.MULTILINE)
|
||||||
|
|
||||||
|
|
||||||
def test_core_version_is_semver():
|
def test_core_version_is_semver():
|
||||||
@@ -89,3 +98,16 @@ def test_web_interface_version_tracks_the_core():
|
|||||||
"web_interface.__version__ has drifted from src.__version__; it should "
|
"web_interface.__version__ has drifted from src.__version__; it should "
|
||||||
"re-export the canonical value rather than hardcode its own."
|
"re-export the canonical value rather than hardcode its own."
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_heading_pattern_is_strict_about_digits_and_whitespace():
|
||||||
|
"""`\\d` also matches non-ASCII decimal digits and `\\s` matches newlines,
|
||||||
|
either of which would let a malformed heading through and then fail the
|
||||||
|
comparison with a confusing message. Pin the tightened patterns."""
|
||||||
|
assert HEADING.findall("## 3.2.0\n") == ["3.2.0"]
|
||||||
|
assert HEADING.findall("##\t3.2.0 \n") == ["3.2.0"]
|
||||||
|
# A bare "##" whose version sits on the next line is not a heading.
|
||||||
|
assert HEADING.findall("##\n3.2.0\n") == []
|
||||||
|
# Arabic-Indic digits parse via int() but are not our version format.
|
||||||
|
assert HEADING.findall("## ٣.٢.٠\n") == []
|
||||||
|
assert SEMVER.match("٣.٢.٠") is None
|
||||||
|
|||||||
Reference in New Issue
Block a user