Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/developer/guides/adversarial-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
46 changes: 45 additions & 1 deletion scripts/triage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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 <N>")
print(f" fixed, waiting on a release -> gh issue edit <N> "
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
Expand Down
Loading