Skip to content

fix(docs): correct the PR template's closing-keyword note (backend#2188) - #594

Merged
saqlainsyed007 merged 2 commits into
developfrom
fix/2188-pr-template-default-branch
Aug 27, 2026
Merged

fix(docs): correct the PR template's closing-keyword note (backend#2188)#594
saqlainsyed007 merged 2 commits into
developfrom
fix/2188-pr-template-default-branch

Conversation

@saqlainsyed007

@saqlainsyed007saqlainsyed007 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Closes tracebloc/backend#2188

Problem

The PR template's line-5 comment carried:

PRs land on develop, not the default branch, so closing keywords do not fire on merge — confirm the issue actually closed.

Both halves are false: developis this repo's default branch, so a same-repo closing keyword does fire on merge (measured 12/12 on recent base:develop merges in backend). A wrong doc is worse than an unhelpful one now that agents read docs first — this exact sentence manufactured a High-severity Bugbot finding on e2e-test-agent#165.

Fix

Replace only the false sentence with the accurate one (as landed in e2e-test-agent#165). The load-bearing cross-repo half — a bare repo#N closes nothing, only an owner-qualified tracebloc/repo#N — is preserved, along with this repo's own example.

One of 8 repos carrying the clause (backend#2188): backend, client, averaging-service, frontend-app, tracebloc-py-package, data-ingestors, design-system, cli.


Note

Low Risk
Documentation-only change to a PR template comment; no runtime, security, or deployment impact.

Overview
Updates the Related section comment in .github/pull_request_template.md so it no longer claims that PRs merge to a non-default branch and that same-repo closing keywords do not work on merge.

The revised text states that develop is this repo's default branch, so same-repo keywords like Closes #123do fire on merge (with a note that this was verified 12/12). The existing guidance on owner-qualified cross-repo references (tracebloc/client-runtime#456 vs a bare client-runtime#456) is unchanged.

Reviewed by Cursor Bugbot for commit 14d2633. Bugbot is set up for automated code reviews on this repo. Configure here.

@LukasWodkaLukasWodka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking on one clause, shared by all nine copies. Full evidence on the canonical, tracebloc/backend#2717.

Short version: the half you fixed is right — develop is the default branch and a same-repo Closes #123 does fire. But the replacement adds a new falsehood:

"A cross-repo one never does, whatever the branch: close those by hand."

An owner-qualified cross-repo keyword fires too. Measured on two merges from today:

e2e-test-agent#291 "Closes tracebloc/backend#2693" merged 14:24:02Z -> #2693 CLOSED 14:24:03Z
tracebloc-py-package#463 -> tracebloc/backend#2646 merged 11:13:49Z -> #2646 CLOSED 11:13:51Z

The real rule is owner-qualification, not same-repo vs cross-repo — which is exactly what the clause you correctly kept already says ("a bare <repo>#456 closes nothing"). The new sentence contradicts it and removes the only reason to owner-qualify.

I'd drop the sentence rather than reword it. CI is green here, so this clause is the only thing holding it.

@saqlainsyed007

Copy link
Copy Markdown
ContributorAuthor

Dropped the false cross-repo one never does clause here too — full context on the canonical backend#2717. Re-requesting review.

@LukasWodkaLukasWodka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — the false clause is gone and what's left is accurate.

I re-extracted the comment line from all nine heads and hashed it: three hashes, differing only in the per-repo example (client-runtime / frontend-app / backend), and "A cross-repo one never does, whatever the branch" is absent from every one. That was the whole blocker.

What stands now is true on each claim: Closes #123 fires because develop is the default branch; a bare<repo>#456 closes nothing because it isn't a reference; owner-qualifying is what makes a cross-repo link real. Dropping the sentence rather than rewriting it was the better call — the parenthetical already carried the rule.

One non-blocking note, offered once and then I'll leave it. The line now states the same-repo case and says nothing about whether an owner-qualified cross-repo keyword fires. It does — I measured it earlier today, e2e-test-agent#291 merged 14:24:02Z and backend#2693 closed 14:24:03Z. Since the entire ticket exists because people believed keywords don't fire, silence on that half is the one place a reader can still guess wrong. "Both fire on merge" would close it. Your call entirely — omission isn't the falsehood I was blocking, and this is already a clear improvement on what was there.

Good turnaround on nine repos.

@saqlainsyed007
saqlainsyed007 merged commit 72b40d9 into developAug 27, 2026
28 checks passed
@saqlainsyed007
saqlainsyed007 deleted the fix/2188-pr-template-default-branch branch August 27, 2026 15:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@saqlainsyed007@LukasWodka