Skip to content

Commit c8523b6

Browse files
authored
Merge pull request #116 from learningequality/pull-request-template-check
Check that a pull request follows the template
2 parents 10fa737 + 81286a9 commit c8523b6

11 files changed

Lines changed: 589 additions & 11 deletions

‎.github/workflows/automation-caller.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@ on: # zizmor: ignore[dangerous-triggers]
2121
- created
2222
pull_request_target:
2323
types:
24+
- edited
2425
- labeled
2526
- opened
27+
- reopened
2628
- review_requested
2729
schedule:
2830
- cron: 1 0 * * 1

‎.github/workflows/automation-group-pull-request-target.yml‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ on:
88
workflow_call:
99
secrets:
1010
LE_BOT_APP_ID:
11-
description: "Forwarded to: review-requested, pull-request-label, dependabot-reviewer, contributor-pr-reply"
11+
description: "Forwarded to: review-requested, pull-request-label, dependabot-reviewer, contributor-pr-reply, pull-request-template"
1212
required: true
1313
LE_BOT_PRIVATE_KEY:
14-
description: "Forwarded to: review-requested, pull-request-label, dependabot-reviewer, contributor-pr-reply"
14+
description: "Forwarded to: review-requested, pull-request-label, dependabot-reviewer, contributor-pr-reply, pull-request-template"
1515
required: true
1616
SLACK_COMMUNITY_NOTIFICATIONS_WEBHOOK_URL:
1717
description: "Forwarded to: contributor-pr-reply"
@@ -54,3 +54,12 @@ jobs:
5454
LE_BOT_APP_ID: ${{ secrets.LE_BOT_APP_ID }}
5555
LE_BOT_PRIVATE_KEY: ${{ secrets.LE_BOT_PRIVATE_KEY }}
5656
SLACK_COMMUNITY_NOTIFICATIONS_WEBHOOK_URL: ${{ secrets.SLACK_COMMUNITY_NOTIFICATIONS_WEBHOOK_URL }}
57+
pull-request-template:
58+
name: Pull Request Template
59+
if: ${{ github.event_name == 'pull_request_target' && contains(fromJSON('["opened","edited","reopened"]'), github.event.action) }}
60+
uses: learningequality/.github/.github/workflows/pull-request-template.yml@main
61+
permissions:
62+
contents: read
63+
secrets:
64+
LE_BOT_APP_ID: ${{ secrets.LE_BOT_APP_ID }}
65+
LE_BOT_PRIVATE_KEY: ${{ secrets.LE_BOT_PRIVATE_KEY }}

‎.github/workflows/automation.yml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ on:
1818
description: Unused - accepted until every consumer stops forwarding it
1919
required: false
2020
LE_BOT_APP_ID:
21-
description: "Forwarded to: review-requested, pull-request-label, dependabot-reviewer, contributor-pr-reply, issue-open, issue-label-header, good-first-issue-comment, community-contribution-label, contributor-issue-comment, unassign-inactive-issues"
21+
description: "Forwarded to: review-requested, pull-request-label, dependabot-reviewer, contributor-pr-reply, pull-request-template, issue-open, issue-label-header, good-first-issue-comment, community-contribution-label, contributor-issue-comment, unassign-inactive-issues"
2222
required: true
2323
LE_BOT_PRIVATE_KEY:
24-
description: "Forwarded to: review-requested, pull-request-label, dependabot-reviewer, contributor-pr-reply, issue-open, issue-label-header, good-first-issue-comment, community-contribution-label, contributor-issue-comment, unassign-inactive-issues"
24+
description: "Forwarded to: review-requested, pull-request-label, dependabot-reviewer, contributor-pr-reply, pull-request-template, issue-open, issue-label-header, good-first-issue-comment, community-contribution-label, contributor-issue-comment, unassign-inactive-issues"
2525
required: true
2626
SLACK_COMMUNITY_NOTIFICATIONS_WEBHOOK_URL:
2727
description: "Forwarded to: contributor-pr-reply, contributor-issue-comment, unassign-inactive-issues"
@@ -32,7 +32,7 @@ on:
3232
jobs:
3333
pull-request-target:
3434
name: Pull Request Target
35-
if: ${{ (github.event_name == 'pull_request_target' && github.event.action == 'review_requested') || (github.event_name == 'pull_request_target' && github.event.action == 'labeled') || (github.event_name == 'pull_request_target' && github.event.action == 'opened' && github.event.pull_request.user.login == 'dependabot[bot]') || (github.event_name == 'pull_request_target' && github.event.action == 'opened') }}
35+
if: ${{ (github.event_name == 'pull_request_target' && github.event.action == 'review_requested') || (github.event_name == 'pull_request_target' && github.event.action == 'labeled') || (github.event_name == 'pull_request_target' && github.event.action == 'opened' && github.event.pull_request.user.login == 'dependabot[bot]') || (github.event_name == 'pull_request_target' && github.event.action == 'opened') || (github.event_name == 'pull_request_target' && contains(fromJSON('["opened","edited","reopened"]'), github.event.action)) }}
3636
uses: learningequality/.github/.github/workflows/automation-group-pull-request-target.yml@main
3737
permissions:
3838
contents: read
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
name: Check that a pull request follows the template
2+
on:
3+
workflow_call:
4+
secrets:
5+
LE_BOT_APP_ID:
6+
description: 'GitHub App ID for authentication'
7+
required: true
8+
LE_BOT_PRIVATE_KEY:
9+
description: 'GitHub App Private Key for authentication'
10+
required: true
11+
jobs:
12+
check-template:
13+
name: Check template
14+
runs-on: ubuntu-latest
15+
steps:
16+
- name: Generate App Token
17+
id: generate-token
18+
uses: actions/create-github-app-token@v3
19+
with:
20+
app-id: ${{ secrets.LE_BOT_APP_ID }}
21+
private-key: ${{ secrets.LE_BOT_PRIVATE_KEY }}
22+
- name: Checkout .github repository
23+
uses: actions/checkout@v7
24+
with:
25+
repository: learningequality/.github
26+
ref: main
27+
token: ${{ steps.generate-token.outputs.token }}
28+
- name: Setup Node.js
29+
uses: actions/setup-node@v7
30+
with:
31+
node-version: '20'
32+
- name: Install dependencies
33+
run: yarn install --frozen-lockfile
34+
- name: Run script
35+
id: script
36+
uses: actions/github-script@v9
37+
with:
38+
github-token: ${{ steps.generate-token.outputs.token }}
39+
script: |
40+
const script = require('./scripts/pull-request-template.js');
41+
return await script({ github, context, core });

‎automation-registry.yml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,16 @@ automations:
6464
LE_BOT_APP_ID: required
6565
LE_BOT_PRIVATE_KEY: required
6666
SLACK_COMMUNITY_NOTIFICATIONS_WEBHOOK_URL: optional
67+
- name: pull-request-template
68+
enabled: true
69+
leaf: pull-request-template.yml
70+
on:
71+
pull_request_target:
72+
types: [opened, edited, reopened]
73+
if: "github.event_name == 'pull_request_target' && contains(fromJSON('[\"opened\",\"edited\",\"reopened\"]'), github.event.action)"
74+
secrets:
75+
LE_BOT_APP_ID: required
76+
LE_BOT_PRIVATE_KEY: required
6777
- name: holiday-message
6878
# Seasonal. The leaf has no date guard, so this must be flipped on before
6979
# the holidays and off after them.

‎automation-template.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@ on: # zizmor: ignore[dangerous-triggers]
2222
- created
2323
pull_request_target:
2424
types:
25+
- edited
2526
- labeled
2627
- opened
28+
- reopened
2729
- review_requested
2830
schedule:
2931
- cron: 1 0 * * 1

‎docs/automation.md‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ contributor replies, issue header management, etc.) copies a single file: `autom
55
There is nothing else to maintain per repo — no per-automation caller, no hand-written `on:` block
66
to keep in sync.
77

8-
The community-contribution spreadsheet is not one of them. `update-pr-spreadsheet.yml` runs daily
9-
in this repo and upserts every public contributor PR in the org updated in the last two days. Run it
10-
by hand with `dry_run` to log the rows without writing, or a larger `days` to backfill.
8+
The community-contribution spreadsheet is not one of those automations. `update-pr-spreadsheet.yml`
9+
runs daily in this repo and upserts every public contributor PR in the org updated in the last two
10+
days. Run it by hand with `dry_run` to log the rows without writing, or a larger `days` to backfill.
1111

1212
## How it fits together
1313

@@ -95,9 +95,9 @@ every repo whose `.github/workflows/automation.yml` calls this repo's `automatio
9595
request targets that repo's default branch, which is the only branch GitHub evaluates workflow
9696
triggers from.
9797

98-
For each consumer it compares the copy with the template and opens a pull request when they differ.
99-
A repo that is already in sync gets nothing, while a repo with an existing sync pull request has
100-
that pull request updated rather than a second one opened.
98+
For each consumer, the workflow compares its copy of `automation.yml` with the template and opens a
99+
pull request if they differ. Repositories with no drift are left unchanged, and existing sync pull
100+
requests are updated rather than duplicated.
101101

102102
The workflow only proposes changes. It opens pull requests on a branch, never commits to a default
103103
branch, and never merges, approves, or enables auto-merge. A core maintainer in each consumer repo

‎docs/community-automations.md‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,30 @@ In `scripts/constants.js` set:
7878
- `BOT_MESSAGE_LINK_ISSUE`: Text added when no issue is linked
7979
- `BOT_MESSAGE_PULL_REQUEST_CLOSED`: Closing message text
8080

81+
# `pull-request-template`
82+
83+
Fails the run when a pull request description does not carry the sections its template requires. It runs on `opened`, `edited` and `reopened`, so a corrected description clears the check. Bot authors are exempt.
84+
85+
The template is resolved the way GitHub resolves it: the repo's own copy first, in all six places GitHub reads them from, and this repo's `.github/PULL_REQUEST_TEMPLATE.md` next. It is read from the pull request's base branch, so a pull request cannot relax the rules it is judged by.
86+
87+
A section is required when its heading carries a `<!-- REQUIRED -->` comment in the template:
88+
89+
```markdown
90+
## Summary
91+
<!-- REQUIRED -->
92+
```
93+
94+
A template that marks no section requires every level-two heading, so that a repo is strict rather than silently unchecked. To change what a repo requires, edit the markers in that repo's template. No change is required here.
95+
96+
Four states fail, and the log names every one of them rather than stopping at the first:
97+
98+
- the heading is missing from the description
99+
- the section holds no text, once HTML comments are stripped
100+
- the section still holds the `…` placeholder
101+
- the section still reads word for word as the template wrote it
102+
103+
The failed check is the only report. Nothing is posted on the pull request.
104+
81105
# `holiday-message`
82106

83107
Sends a holiday message to community pull requests and issue comments.

‎scripts/constants.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,20 @@ const PR_STATS_REPOS = [
167167
// Repositories in which we accept open-source contributions
168168
const COMMUNITY_REPOS = [...PR_STATS_REPOS];
169169

170+
// Every place GitHub reads a pull request template from. The contents API is
171+
// case sensitive, so both spellings are needed.
172+
const PR_TEMPLATE_PATHS = [
173+
'.github/pull_request_template.md',
174+
'.github/PULL_REQUEST_TEMPLATE.md',
175+
'pull_request_template.md',
176+
'PULL_REQUEST_TEMPLATE.md',
177+
'docs/pull_request_template.md',
178+
'docs/PULL_REQUEST_TEMPLATE.md',
179+
];
180+
181+
// This repo's own template, which applies to a repo holding none of its own.
182+
const PR_TEMPLATE_TARGET = '.github/PULL_REQUEST_TEMPLATE.md';
183+
170184
module.exports = {
171185
LE_BOT_USERNAME,
172186
BOT_USERNAMES,
@@ -195,4 +209,6 @@ module.exports = {
195209
HOLIDAY_MESSAGE,
196210
PR_STATS_REPOS,
197211
COMMUNITY_REPOS,
212+
PR_TEMPLATE_PATHS,
213+
PR_TEMPLATE_TARGET,
198214
};

‎scripts/pull-request-template.js‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/**
2+
* Fails the run when a pull request body does not carry the sections that its
3+
* template marks as required.
4+
*/
5+
6+
const fs = require('fs');
7+
const path = require('path');
8+
9+
const { isBot } = require('./utils');
10+
const { PR_TEMPLATE_PATHS, PR_TEMPLATE_TARGET } = require('./constants');
11+
12+
const ORG_TEMPLATE_PATH = path.join(__dirname, '..', PR_TEMPLATE_TARGET);
13+
14+
const HEADING = /^## [^#]/;
15+
const REQUIRED_MARKER = /^\s*<!--\s*REQUIRED\s*-->\s*$/;
16+
const HTML_COMMENT = /<!--[\s\S]*?-->/g;
17+
const PLACEHOLDER = '…';
18+
19+
function sections(markdown) {
20+
const out = [];
21+
for (const line of (markdown || '').split(/\r?\n/)) {
22+
if (HEADING.test(line)) out.push({ heading: line.trim(), lines: [] });
23+
else if (out.length) out[out.length - 1].lines.push(line);
24+
}
25+
return out;
26+
}
27+
28+
function isMarked(section) {
29+
const first = section.lines.find(line => line.trim() !== '');
30+
return first !== undefined && REQUIRED_MARKER.test(first);
31+
}
32+
33+
// A template that marks nothing requires every section, so a repo whose template
34+
// has no markers is strict rather than unchecked.
35+
function requiredSections(template) {
36+
const all = sections(template);
37+
const marked = all.filter(isMarked);
38+
return marked.length ? marked : all;
39+
}
40+
41+
function content(section) {
42+
return section.lines.join('\n').replace(HTML_COMMENT, '').trim();
43+
}
44+
45+
function failures(body, required) {
46+
const found = new Map(sections(body).map(section => [section.heading.toLowerCase(), section]));
47+
const problems = [];
48+
for (const { heading, lines } of required) {
49+
const section = found.get(heading.toLowerCase());
50+
if (!section) {
51+
problems.push(`\`${heading}\` is missing.`);
52+
continue;
53+
}
54+
const text = content(section);
55+
if (text === '') {
56+
problems.push(`\`${heading}\` is empty.`);
57+
} else if (text.split('\n').some(line => line.trim() === PLACEHOLDER)) {
58+
// The template puts the placeholder on a line of its own, so an ellipsis
59+
// inside a sentence is prose rather than an unfilled section.
60+
problems.push(`\`${heading}\` still holds the \`${PLACEHOLDER}\` placeholder.`);
61+
} else if (text === content({ lines })) {
62+
problems.push(`\`${heading}\` still holds the text from the template.`);
63+
}
64+
}
65+
return problems;
66+
}
67+
68+
/**
69+
* The repo's own template, `null` if none exists, or `{ unreadable }` if a
70+
* candidate path returns anything other than a 404. An unreadable path may
71+
* contain a repo-owned template, so validating against the shared template
72+
* could reject a body that follows the template the author was given.
73+
*/
74+
async function readRepoTemplate({ github, context, core }, ref) {
75+
for (const templatePath of PR_TEMPLATE_PATHS) {
76+
try {
77+
const response = await github.rest.repos.getContent({
78+
owner: context.repo.owner,
79+
repo: context.repo.repo,
80+
path: templatePath,
81+
ref,
82+
});
83+
if (response.data?.content) {
84+
core.info(`Checking against ${templatePath} on ${ref}.`);
85+
return { template: Buffer.from(response.data.content, 'base64').toString('utf8') };
86+
}
87+
} catch (error) {
88+
if (error.status !== 404) {
89+
return { unreadable: `${templatePath}: ${error.message}` };
90+
}
91+
}
92+
}
93+
return { template: null };
94+
}
95+
96+
module.exports = async ({ github, context, core }) => {
97+
try {
98+
const pullRequest = context.payload.pull_request;
99+
const author = pullRequest.user.login;
100+
101+
if (await isBot(author, { core })) {
102+
core.info(`${author} is a bot, so the template is not checked.`);
103+
return;
104+
}
105+
106+
const found = await readRepoTemplate({ github, context, core }, pullRequest.base.ref);
107+
if (found.unreadable) {
108+
core.warning(`Could not read ${found.unreadable}, so the template is not checked.`);
109+
return;
110+
}
111+
112+
const template = found.template ?? fs.readFileSync(ORG_TEMPLATE_PATH, 'utf8');
113+
if (!found.template) {
114+
core.info('No template in this repo, so the shared template applies.');
115+
}
116+
117+
const required = requiredSections(template);
118+
if (!required.length) {
119+
core.warning('The template holds no sections, so there is nothing to check.');
120+
return;
121+
}
122+
123+
const problems = failures(pullRequest.body, required);
124+
if (!problems.length) {
125+
core.info(`The body carries all ${required.length} required sections.`);
126+
return;
127+
}
128+
129+
for (const problem of problems) {
130+
core.error(problem);
131+
}
132+
core.setFailed(
133+
`The description does not follow the pull request template:\n` +
134+
problems.map(problem => `- ${problem}`).join('\n') +
135+
`\n\nRequired: ${required.map(section => section.heading).join(', ')}. ` +
136+
`Edit the description and the check runs again.`,
137+
);
138+
} catch (error) {
139+
core.setFailed(`Action failed with error: ${error.message}`);
140+
}
141+
};

0 commit comments

Comments
 (0)