diff --git a/docs/developer/guides/adversarial-review.md b/docs/developer/guides/adversarial-review.md index f05386aa..1d281f48 100644 --- a/docs/developer/guides/adversarial-review.md +++ b/docs/developer/guides/adversarial-review.md @@ -119,6 +119,18 @@ on the PR with its evidence: | **Already fixed** | it was fixed earlier and nobody closed it | a probe run against `development`, pasted into the issue as the closing comment, showing the behaviour the issue describes no longer happens | | **Still live** | the branch passes through it and leaves it | one line saying which part still reproduces, so the next session does not re-derive it | +**`fixed-in-development` carries an issue from merge to release.** GitHub +closes a linked issue when its PR reaches the *default* branch, which here is +`main`, while the work merges to `development`. Merges to `main` are +infrequent, so between the two an issue is fixed and still open and nothing +says so. Across the project's life 139 declared closes produced only 5 that +slipped, so the mechanism works — it just works at release time. Label the +issue at merge, and the release closes everything carrying the label in one +pass. Closing it at merge instead is a reasonable choice for a defect nobody +outside is waiting on, but it reads as a lie to anyone running the release. +`scripts/triage.py` lists every issue a merged PR declared that is neither +closed nor labelled. + **Refusing to close is a verdict, not a gap.** A branch that touches the territory of an issue and does not fix it says so, in a line. The failure this replaces is silence: the fix lands, the issue reads as familiar to anyone who diff --git a/scripts/triage.py b/scripts/triage.py index 3f091de6..e9483bba 100755 --- a/scripts/triage.py +++ b/scripts/triage.py @@ -44,6 +44,11 @@ def prose(body): FAILED = re.compile(r"^FAILED\s+(\S+?)::(\S+?)(?:\[|\s|$)") BASE = "development" +#: Merges to `main` are infrequent, so GitHub's own issue-closing (which fires +#: only on a default-branch merge) lags reality by a release. An issue whose fix +#: is on `development` carries this label until then: closing it would be a lie +#: to anyone running the release, and leaving it bare loses the fact entirely. +FIXED_LABEL = "fixed-in-development" def sh(*args, check=True, tries=1): @@ -83,7 +88,7 @@ def issues(): def prs(state="open"): fields = ("number,title,mergeable,additions,deletions,changedFiles," - "statusCheckRollup,body,headRefName,author,updatedAt") + "statusCheckRollup,body,headRefName,baseRefName,author,updatedAt") args = ["pr", "list", "--limit", "400", "--json", fields] if state != "open": args += ["--state", state] @@ -219,6 +224,45 @@ def main(): no_closes = [p for p in opn if not CLOSES.search(prose(p["body"]))] print(f" {len(no_closes)} of {len(opn)} carry no Closes line") + # ---- the gap between "merged" and "released" --------------------------- + # GitHub closes a linked issue when the PR reaches the DEFAULT branch, and + # this project's default branch is `main` while the work merges to + # `development`. Across the project's life 139 declared closes produced 5 + # that slipped, so the mechanism does work -- but it works at release time, + # and releases are rare. Between a merge and a release an issue is fixed + # and still open, and nothing says so unless somebody labels it. + labels = {i["number"]: {l["name"] for l in i["labels"]} for i in iss} + declared = {} + for p in merged: + if p.get("baseRefName") not in (None, BASE): + continue + for n in {int(m) for m in CLOSES.findall(prose(p["body"]))}: + if n in open_numbers: + declared.setdefault(n, []).append(p["number"]) + limbo = {n: v for n, v in declared.items() if FIXED_LABEL not in labels.get(n, ())} + print(f"\nFIXED BUT STILL OPEN (a merged PR declared it; `{FIXED_LABEL}` not applied)") + if not limbo: + print(f" none -- every declared close is either closed or labelled") + for n in sorted(limbo): + src = ", ".join(f"#{p}" for p in sorted(limbo[n])) + print(f" #{n:<5} declared by {src:<14} {titles[n][:58]}") + if limbo: + print("\n Probe each. Three outcomes, and the third is why this is a list") + print(" of candidates rather than a list of fixes:") + print(f" fixed and you want it off the board -> gh issue close ") + print(f" fixed, waiting on a release -> gh issue edit " + f"--add-label {FIXED_LABEL}") + print(" NOT fixed -- the PR addressed a neighbour, or papered over it") + print(" -> leave open, say which part is live") + print(" #611 is the standing example of the third: #656 was credited with") + print(" closing it, and the hang is handled by a --deselect in scripts/test.sh") + print(" at the very rank count the issue reports.") + carrying = [n for n, ls in labels.items() if FIXED_LABEL in ls] + if carrying: + print(f"\n {len(carrying)} issue(s) already carry `{FIXED_LABEL}` and close at " + f"the next release:") + print(" " + " ".join(f"#{n}" for n in sorted(carrying))) + # ---- the one that cost ten days --------------------------------------- if args.no_logs: return