diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 7fc1a91a..4060b94f 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -7,6 +7,7 @@ on: permissions: contents: read issues: read + pull-requests: read jobs: lint: @@ -24,9 +25,9 @@ jobs: - name: Enforce em-dash-free rule + check for broken doc links + ruff run: make lint - # Roadmap / changelog issue numbers against GitHub (#963). Informational for now: it can go red - # between a contributor closing an issue and the roadmap being updated, which should not block a merge. - - name: Check issue numbers cited in ROADMAP.md and CHANGELOG.md + # Issue and PR numbers against GitHub (#963, #981). Informational for now: it can go red + # between a contributor closing an issue and the docs being updated, which should not block a merge. + - name: Check issue numbers cited in docs (ROADMAP, CHANGELOG, CONTRIBUTORS, METRICS) run: python scripts/check_issue_refs.py --require env: GH_TOKEN: ${{ github.token }} diff --git a/Makefile b/Makefile index 93abd7c1..aecddae8 100644 --- a/Makefile +++ b/Makefile @@ -39,7 +39,7 @@ favicons: ## Regenerate favicon.ico/PNGs and apple-touch-icon.png from logo.svg fix-explainer-count: ## Correct stale "N explainers" mentions in README/CONTRIBUTORS/METRICS/ROADMAP $(PY) scripts/check_explainer_count.py --fix -lint: ## Enforce the em-dash-free rule + check for broken doc links + issue numbers in ROADMAP/CHANGELOG (needs gh, skipped offline) + ruff (mirrors the lint workflow) +lint: ## Enforce the em-dash-free rule + check for broken doc links + issue numbers in docs (needs gh, skipped offline) + ruff (mirrors the lint workflow) $(PY) scripts/check_em_dash.py $(PY) scripts/check_broken_links.py $(PY) scripts/check_issue_refs.py diff --git a/scripts/check_issue_refs.py b/scripts/check_issue_refs.py index 736fdac3..99ccda4d 100644 --- a/scripts/check_issue_refs.py +++ b/scripts/check_issue_refs.py @@ -1,16 +1,18 @@ #!/usr/bin/env python3 -"""Fail when the issue numbers cited in the roadmap and changelog disagree with GitHub (#963). +"""Fail when issue numbers cited in docs disagree with GitHub (#963, #981). -The roadmap, changelog and metrics cite issue numbers by hand and have been wrong before (two -sweeps listed the slow-test marker and the held-out encodings under each other's numbers). Two -rules, applied to issues only (a number that is not an issue, such as a pull request, is ignored): +The roadmap, changelog, contributors and metrics cite issue numbers by hand and have been wrong +before (two sweeps listed the slow-test marker and the held-out encodings under each other's +numbers). Rules: * ROADMAP.md: a checked item (`- [x]`) may cite only closed issues, and an unchecked item (`- [ ]`) must cite at least one open issue (unless it cites none at all); - * CHANGELOG.md: every `closes #N` / `(closes #N, #M)` must name a closed issue. + * CHANGELOG.md: every `closes #N` / `(closes #N, #M)` must name a closed issue; + * CONTRIBUTORS.md: every `closing issue #N` / `closing #N` must name a closed issue; + * METRICS.md: every `#N` in a sweep note (`> ...`) must be an existing issue or pull request. - python3 scripts/check_issue_refs.py # asks `gh` for the issue states - python3 scripts/check_issue_refs.py --issues FILE # FILE: JSON [{"number": N, "state": "OPEN"|"CLOSED"}] + python3 scripts/check_issue_refs.py # asks `gh` for the issue and PR states + python3 scripts/check_issue_refs.py --issues FILE # FILE: JSON [{"number": N, "state": "OPEN"|"CLOSED"|"MERGED"}] Without `gh` (or a network) it prints a note and exits 0, so `make lint` works offline; pass `--require` to make that an error (CI). @@ -29,10 +31,11 @@ ROOT = Path(__file__).resolve().parent.parent REF = re.compile(r"#(\d+)") CLOSES = re.compile(r"closes? ((?:#\d+(?:,? (?:and )?)?)+)", re.IGNORECASE) +CLOSING = re.compile(r"closing\s+(?:issues?\s+)?((?:#\d+(?:(?:,| and|-)\s*)?)+)", re.IGNORECASE) def load_states(issues_file: str | None) -> dict[int, str] | None: - """{issue number: "open" | "closed"}, or None when GitHub cannot be asked.""" + """{issue/PR number: "open" | "closed" | "merged"}, or None when GitHub cannot be asked.""" if issues_file: data = json.loads(Path(issues_file).read_text(encoding="utf-8")) else: @@ -43,6 +46,10 @@ def load_states(issues_file: str | None) -> dict[int, str] | None: if done.returncode != 0: return None data = json.loads(done.stdout) + prs = subprocess.run(["gh", "pr", "list", "--state", "all", "--limit", "5000", "--json", "number,state"], + capture_output=True, text=True, cwd=ROOT) + if prs.returncode == 0: + data.extend(json.loads(prs.stdout)) return {int(i["number"]): str(i["state"]).lower() for i in data} @@ -61,7 +68,7 @@ def check_roadmap(text: str, states: dict[int, str]) -> list[str]: if open_ones: problems.append(f"ROADMAP.md:{number}: checked item cites open issue(s) " + ", ".join(f"#{n}" for n in open_ones)) - elif all(states[n] == "closed" for n in cited): + elif all(states[n] in ("closed", "merged") for n in cited): problems.append(f"ROADMAP.md:{number}: unchecked item cites only closed issue(s) " + ", ".join(f"#{n}" for n in cited)) return problems @@ -78,6 +85,29 @@ def check_changelog(text: str, states: dict[int, str]) -> list[str]: return problems +def check_contributors(text: str, states: dict[int, str]) -> list[str]: + problems = [] + for match in CLOSING.finditer(text): + number = text[:match.start()].count("\n") + 1 + for ref in REF.findall(match.group(1)): + n = int(ref) + if n in states and states[n] == "open": + problems.append(f"CONTRIBUTORS.md:{number}: 'closing #{n}' but #{n} is still open") + return problems + + +def check_metrics(text: str, states: dict[int, str]) -> list[str]: + problems = [] + for number, line in enumerate(text.splitlines(), 1): + if not line.lstrip().startswith(">"): + continue + for ref in REF.findall(line): + n = int(ref) + if n not in states: + problems.append(f"METRICS.md:{number}: '#{n}' is not an existing issue or pull request") + return problems + + def main(argv: list[str] | None = None) -> int: parser = argparse.ArgumentParser(description=__doc__.split("\n")[0]) parser.add_argument("--issues", metavar="FILE", help="issue states as JSON instead of asking gh") @@ -87,14 +117,22 @@ def main(argv: list[str] | None = None) -> int: if states is None: print("note: could not fetch issue states from GitHub (gh missing or offline); skipping the issue-reference check") return 2 if args.require else 0 - problems = (check_roadmap((ROOT / "ROADMAP.md").read_text(encoding="utf-8"), states) - + check_changelog((ROOT / "CHANGELOG.md").read_text(encoding="utf-8"), states)) + problems = [] + for filename, checker in ( + ("ROADMAP.md", check_roadmap), + ("CHANGELOG.md", check_changelog), + ("CONTRIBUTORS.md", check_contributors), + ("METRICS.md", check_metrics), + ): + path = ROOT / filename + if path.is_file(): + problems.extend(checker(path.read_text(encoding="utf-8"), states)) for problem in problems: print(problem) if problems: print(f"{len(problems)} issue reference(s) disagree with GitHub") return 1 - print(f"OK: issue references in ROADMAP.md and CHANGELOG.md agree with GitHub ({len(states):,} issues checked)") + print(f"OK: issue references in ROADMAP.md, CHANGELOG.md, CONTRIBUTORS.md and METRICS.md agree with GitHub ({len(states):,} issues checked)") return 0 diff --git a/tests/test_check_issue_refs.py b/tests/test_check_issue_refs.py index a2e64a87..5270c595 100644 --- a/tests/test_check_issue_refs.py +++ b/tests/test_check_issue_refs.py @@ -1,11 +1,11 @@ -"""scripts/check_issue_refs.py compares roadmap/changelog issue numbers with GitHub (#963).""" +"""scripts/check_issue_refs.py compares roadmap/changelog issue numbers with GitHub (#963, #981).""" import importlib import json script = importlib.import_module("scripts.check_issue_refs") -STATES = {1: "closed", 2: "closed", 3: "open", 4: "open"} +STATES = {1: "closed", 2: "closed", 3: "open", 4: "open", 5: "merged"} def test_checked_roadmap_items_may_cite_only_closed_issues(): @@ -32,6 +32,33 @@ def test_changelog_closes_must_name_closed_issues(): ] +def test_contributors_closing_must_name_closed_issues(): + text = ( + "- Alice ([#10](...), closing issue #1)\n" + "- Bob ([#11](...), closing issue #3)\n" + "- Carol ([#12](...), closing #4 and #2)\n" + "- Dave ([#13](...), closing issues #1, #3)\n" + "- mentions #3 without closing\n" + ) + assert script.check_contributors(text, STATES) == [ + "CONTRIBUTORS.md:2: 'closing #3' but #3 is still open", + "CONTRIBUTORS.md:3: 'closing #4' but #4 is still open", + "CONTRIBUTORS.md:4: 'closing #3' but #3 is still open", + ] + + +def test_metrics_sweep_notes_must_cite_existing_issues_or_prs(): + text = ( + "# Metrics\n\n" + "> **2026-W42 - sweep:** closed issues (#1, #2, #5)\n" + "> **2026-W43 - next sweep:** closed issues (#1, #999)\n" + "Regular prose citing #8888 is not a sweep note\n" + ) + assert script.check_metrics(text, STATES) == [ + "METRICS.md:4: '#999' is not an existing issue or pull request", + ] + + def test_main_reads_an_issues_file_and_reports(tmp_path, capsys, monkeypatch): issues = tmp_path / "issues.json" issues.write_text(json.dumps([{"number": n, "state": s.upper()} for n, s in STATES.items()])) @@ -45,7 +72,7 @@ def test_main_reads_an_issues_file_and_reports(tmp_path, capsys, monkeypatch): assert "checked item cites open issue(s) #3" in out and "1 issue reference(s) disagree" in out roadmap.write_text("- [x] a (#1)\n") assert script.main(["--issues", str(issues)]) == 0 - assert "agree with GitHub (4 issues checked)" in capsys.readouterr().out + assert "agree with GitHub (5 issues checked)" in capsys.readouterr().out def test_without_gh_the_check_is_skipped_unless_required(capsys, monkeypatch):