Skip to content

Commit e7284ed

Browse files
authored
Gate draft PRs too and rewrite the auto-close comment (#3378)
1 parent 0921d94 commit e7284ed

3 files changed

Lines changed: 38 additions & 24 deletions

File tree

.github/scripts/pr_intake_gate.js

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,10 @@ module.exports = async function run({ github, context, core }) {
7878
if (pr.merged_at) return log('merged — nothing to do');
7979
if (pr.state === 'closed' && !gated) return log('closed by someone else — not ours');
8080

81-
// 1. Exempt authors: bots, anyone with triage or better, and drafts (which
82-
// are checked again on ready_for_review).
81+
// 1. Exempt authors: bots and anyone with triage or better. Drafts are
82+
// gated like any other PR.
8383
if (pr.user.type === 'Bot') return log('author is a bot — exempt');
8484
if (await isTrusted(pr.user.login)) return pass('author has triage+ on this repo');
85-
if (pr.draft) return log('draft — skipped until ready for review');
8685

8786
// 2. Overrides: a triage+ user reopening the PR or removing the label wants
8887
// it open. Anyone else doing so just triggers a re-check.
@@ -122,7 +121,7 @@ module.exports = async function run({ github, context, core }) {
122121
async function fail(linkedIssues) {
123122
console.log(`FAIL: ${linkedIssues.length ? `not assigned to ${linkedIssues.map((n) => `#${n}`).join(', ')}` : 'no usable issue link'}`);
124123
await addLabel(prNumber, LABEL);
125-
await upsertGateComment(prNumber, closedComment(linkedIssues));
124+
await upsertGateComment(prNumber, closedComment(pr.draft, linkedIssues));
126125
if (pr.state === 'open') {
127126
await mutate(`close PR #${prNumber}`, () => github.rest.pulls.update({ owner, repo, pull_number: prNumber, state: 'closed' }));
128127
}
@@ -135,23 +134,37 @@ module.exports = async function run({ github, context, core }) {
135134

136135
// ── Comment text ─────────────────────────────────────────────────────────
137136

138-
function closedComment(linkedIssues) {
137+
function closedComment(draft, linkedIssues) {
139138
const issues = linkedIssues.map((n) => `#${n}`).join(', ');
140-
const why = linkedIssues.length
141-
? `you aren't currently assigned to ${issues}`
142-
: "its description doesn't yet link an open issue in this repository (with `Fixes #123` or similar)";
143-
const next = linkedIssues.length
144-
? `If a maintainer would like this change as a PR from you, they'll assign you to ${issues} and this PR will reopen automatically — there's nothing more you need to do. (If you opened the issue, this PR already shows up on its timeline.)`
145-
: `If there isn't an issue for this yet, please [open one](https://github.com/${owner}/${repo}/issues/new/choose) — a clear description of the problem is genuinely the most useful thing for us. Then add \`Fixes #<number>\` to this PR's description. If a maintainer would like the change as a PR from you, they'll assign you to the issue and this PR will reopen automatically.`;
139+
const opener = draft
140+
? "This PR has been closed automatically. It's still a draft, but we close those early so you don't put in more time only to have it closed the moment you mark it ready.\n\n"
141+
: 'This PR has been closed automatically. ';
142+
const rule = `${opener}This repo only keeps pull requests open when they come from a maintainer, or from a contributor a maintainer has assigned to the linked issue`;
143+
const situation = linkedIssues.length
144+
? [
145+
`${rule}, and you aren't currently assigned to ${issues}.`,
146+
'',
147+
`If a maintainer assigns you to ${issues}, this PR reopens on its own and there's nothing more you need to do here. Assignment is a maintainer call based on capacity; comments that only ask to be assigned don't factor in. What does help is engaging on the issue itself by confirming the repro, explaining why it matters for your use case, or describing the approach you'd take.`,
148+
]
149+
: [
150+
`${rule}, and this PR doesn't link an open issue yet.`,
151+
'',
152+
"- **If you're already assigned to an issue for this**, add `Fixes #<n>` to the description and the PR will reopen on its own.",
153+
`- **If there's no issue yet**, please [open one](https://github.com/${owner}/${repo}/issues/new/choose) instead: what you ran into, why it matters for your use case, and a minimal reproduction. That context is super important to us and is what we use to decide what to prioritise.`,
154+
"- **If there's an issue but you're not assigned**, add `Fixes #<n>` anyway so they're linked, then engage on the issue itself by confirming the repro or describing the approach you'd take. Assignment is a maintainer call based on capacity; comments that only ask to be assigned don't factor in. If you are assigned, this PR reopens automatically.",
155+
];
146156
return [
147157
MARKER,
148-
`Thanks for the contribution. This repository only keeps pull requests open when they're linked to an issue that a maintainer has assigned to the author — [CONTRIBUTING.md](${contributingUrl}) explains why and how we work. This PR has been closed for now because ${why}.`,
158+
...situation,
149159
'',
150-
next,
160+
"You're welcome to keep pushing commits here (just avoid force-pushing, since GitHub can't reopen a rewritten branch), but that on its own won't get the PR reviewed or the issue assigned, and realistically most auto-closed PRs stay closed. There's no need to open a new PR either way.",
151161
'',
152-
"There's no need to open a new PR — this one will be reopened. While it's closed, please push any updates as new commits rather than force-pushing, since GitHub can't reopen a PR whose branch has been rewritten.",
162+
`[CONTRIBUTING.md](${contributingUrl}) has the full reasoning, but in short:`,
153163
'',
154-
`*Maintainers: reopening this PR, removing the \`${LABEL}\` label, or adding \`${BYPASS_LABEL}\` bypasses the check.*`,
164+
"- We're a small team with very little capacity to review community PRs right now.",
165+
'- Many recent PRs are AI-generated with little human review, and reviewing one carefully still costs a maintainer as much time as it ever did. A well-described issue is usually more useful to us than the code.',
166+
'',
167+
`*Maintainers: reopen, remove \`${LABEL}\`, or add \`${BYPASS_LABEL}\` to override.*`,
155168
].join('\n');
156169
}
157170

.github/scripts/pr_intake_gate.test.js

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ const PEOPLE = {
2828
// ── Scenarios ──────────────────────────────────────────────────────────────
2929
// `prs` / `issues` describe the world before the event; `expect` describes each
3030
// PR afterwards: state, labels, and comment ('closed' = the "this PR has been
31-
// closed" comment, 'cannot-reopen' = the refused-reopen comment, null = none).
31+
// closed" comment, 'closed-draft' = its draft wording, 'cannot-reopen' = the
32+
// refused-reopen comment, null = none).
3233
// `writes: 0` additionally asserts the gate touched nothing at all.
3334

3435
const scenarios = [
@@ -146,14 +147,13 @@ const scenarios = [
146147
writes: 0,
147148
},
148149
{
149-
name: 'draft PR is skipped until it is marked ready for review',
150+
name: 'draft PR with no issue link is closed like any other, with a note about why drafts are closed early',
150151
prs: [pr(3300, 'outsider', { draft: true })],
151152
event: opened(3300, 'outsider'),
152-
expect: { 3300: { state: 'open', labels: [], comment: null } },
153-
writes: 0,
153+
expect: { 3300: { state: 'closed', labels: [LABEL], comment: 'closed-draft' } },
154154
},
155155
{
156-
name: 'draft marked ready for review with no link → closed',
156+
name: 'pre-existing draft marked ready for review with no link → closed',
157157
prs: [pr(3300, 'outsider')],
158158
event: readyForReview(3300, 'outsider'),
159159
expect: { 3300: { state: 'closed', labels: [LABEL], comment: 'closed' } },
@@ -299,7 +299,8 @@ function observe(world, expect) {
299299
const p = world.prs.get(Number(num));
300300
const gateComments = p.comments.filter((c) => c.user === 'github-actions[bot]' && c.body.includes('<!-- require-linked-issue -->'));
301301
assert.ok(gateComments.length <= 1, `PR #${num} has ${gateComments.length} gate comments`);
302-
const kind = !gateComments.length ? null : gateComments[0].body.includes("won't let it be reopened") ? 'cannot-reopen' : 'closed';
302+
const body = gateComments[0]?.body;
303+
const kind = !body ? null : body.includes("won't let it be reopened") ? 'cannot-reopen' : body.includes('still a draft') ? 'closed-draft' : 'closed';
303304
out[num] = { state: p.state, labels: [...p.labels].sort(), comment: kind };
304305
if ('foreignComments' in expect[num]) out[num].foreignComments = p.comments.length - gateComments.length;
305306
}

.github/workflows/require-linked-issue.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
# In short: a PR from someone without triage rights stays open only if it links
55
# an open issue here that is assigned to them (or labeled `help wanted`);
66
# otherwise it is labeled `missing-issue-link`, gets one comment, and is closed,
7-
# and it reopens automatically once the author is assigned. Bots and drafts are
8-
# skipped. A triage+ user reopening the PR, removing the label, or adding
9-
# `bypass-issue-check` overrides.
7+
# and it reopens automatically once the author is assigned. Drafts are gated
8+
# too; bots are skipped. A triage+ user reopening the PR, removing the label,
9+
# or adding `bypass-issue-check` overrides.
1010
#
1111
# Operating it:
1212
# - Live by default. To pause it without a revert, set the repository

0 commit comments

Comments
 (0)