diff --git a/docs/developer/guides/adversarial-review.md b/docs/developer/guides/adversarial-review.md index f05386aa..b6115d86 100644 --- a/docs/developer/guides/adversarial-review.md +++ b/docs/developer/guides/adversarial-review.md @@ -119,6 +119,24 @@ 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-PR` says the fix is written.** Two situations share the label and +neither is visible without it. A PR merged to `development` has not closed its +issue, because GitHub closes on a *default*-branch merge and that is `main`, +which this project reaches infrequently; across its life 139 declared closes +produced only 5 that slipped, so the mechanism works — it works at release +time. And a PR that has not merged at all can hold the only fix there is: +#785 carries #783 and #784 and cannot merge, its base branch having no PR of +its own. On the day the label was added, four of the five issues it applied to +were in that second state, not the first. + +The label does not record which situation, or which PR. That belongs in the +issue and in `scripts/triage.py`, which lists every issue a PR declares that +is neither closed nor labelled, and marks each PR merged or open. A label +cannot be kept true; a line in the issue can. + +Closing at merge instead of labelling is a reasonable choice for a defect +nobody outside is waiting on. It reads as a lie to anyone running the release. + **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..e1307cb6 100755 --- a/scripts/triage.py +++ b/scripts/triage.py @@ -44,6 +44,17 @@ def prose(body): FAILED = re.compile(r"^FAILED\s+(\S+?)::(\S+?)(?:\[|\s|$)") BASE = "development" +#: An issue whose fix lives in a PR carries this until the fix is released. +#: Two situations share it, and both are invisible without it. A PR merged to +#: `development` has not closed its issue, because GitHub closes on a +#: DEFAULT-branch merge and that is `main`, which this project reaches +#: infrequently. And a PR that has not merged at all may hold the only fix +#: there is -- #785 carries the fix for #783 and #784 and cannot merge, +#: because its base branch has no PR of its own. +#: +#: The label does not say which of the two, or which PR. That belongs in the +#: issue and in this tool's output, where it can be kept true; a label cannot. +FIXED_LABEL = "fixed-in-PR" def sh(*args, check=True, tries=1): @@ -83,7 +94,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 +230,51 @@ 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") + # ---- issues whose fix is already written ------------------------------ + # Two ways an open issue can already be fixed, and neither is visible + # anywhere. A PR merged to `development` has not closed it, because GitHub + # closes on a DEFAULT-branch merge and that is `main` (139 declared closes + # across the project's life, 5 slipped -- release-gated, not broken). And + # an UNMERGED PR can hold the only fix: #785 carries #783 and #784 and + # cannot merge, its base branch having no PR of its own. + 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"], "merged")) + for p in opn: + for n in {int(m) for m in CLOSES.findall(prose(p["body"]))}: + if n in open_numbers: + declared.setdefault(n, []).append((p["number"], "open")) + limbo = {n: v for n, v in declared.items() if FIXED_LABEL not in labels.get(n, ())} + print(f"\nFIX ALREADY WRITTEN (a PR declares it; `{FIXED_LABEL}` not applied)") + if not limbo: + print(" none -- every declared fix is closed or labelled") + for n in sorted(limbo): + src = ", ".join(f"#{p} ({st})" for p, st in sorted(limbo[n])) + print(f" #{n:<5} {src:<26} {titles[n][:52]}") + 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.") + print(" A PR marked (open) is the commoner case here -- the fix is written") + print(" and cannot land. Label the issue; the work is on the PR.") + carrying = [n for n, ls in labels.items() if FIXED_LABEL in ls] + if carrying: + print(f"\n {len(carrying)} issue(s) carry `{FIXED_LABEL}`; the release closes " + f"the ones whose PR has merged:") + print(" " + " ".join(f"#{n}" for n in sorted(carrying))) + # ---- the one that cost ten days --------------------------------------- if args.no_logs: return