Skip to content

Commit 51ad347

Browse files
Replace Danger PR review with native GitHub Actions script (#93)
* Replace website Danger workflow with native reviewer * Fix reviewer spelling lint
1 parent 1fd6e45 commit 51ad347

10 files changed

Lines changed: 196 additions & 2036 deletions

File tree

‎.github/scripts/pr-review.mjs‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
import { appendFileSync, readFileSync } from 'node:fs';
2+
import { pathToFileURL } from 'node:url';
3+
4+
const SMALL_PR_FILES = 10;
5+
const SMALL_PR_LINES = 200;
6+
const requiredSections = ['## Description', '## Type of Change', '## How Has This Been Tested?', '## Checklist'];
7+
const checklistItems = [
8+
'My code follows the style guidelines of this project',
9+
'I have performed a self-review of the code',
10+
'I have commented my code, particularly in hard-to-understand areas',
11+
'I have made corresponding changes to the documentation',
12+
'My changes generate no new warnings',
13+
'Any dependent changes have been merged and published in downstream modules',
14+
'I have checked my code and corrected any misspellings',
15+
];
16+
17+
function isAddedOrModified(file) {
18+
return ['added', 'modified', 'renamed', 'copied'].includes(file.status);
19+
}
20+
21+
export function reviewPullRequest(pullRequest, files) {
22+
const failures = [];
23+
const warnings = [];
24+
const messages = [];
25+
const body = pullRequest.body ?? '';
26+
27+
if (!body) {
28+
failures.push(
29+
'Missing Summary - Add a `## Description` section explaining the motivation, changes, related issue, and required dependencies.',
30+
);
31+
}
32+
if (!pullRequest.title) failures.push('Missing PR Title - Add a relevant pull request title.');
33+
34+
for (const section of requiredSections) {
35+
if (!body.includes(section)) failures.push(`Missing section: ${section}`);
36+
}
37+
for (const item of checklistItems) {
38+
if (!body.includes(`- [x] ${item}`)) warnings.push(`Unchecked checklist item: ${item}`);
39+
}
40+
41+
const additions = files.reduce((total, file) => total + (file.additions ?? 0), 0);
42+
const deletions = files.reduce((total, file) => total + (file.deletions ?? 0), 0);
43+
const changedLines = additions + deletions;
44+
45+
if (additions < deletions) messages.push('Thanks for removing more lines than you add.');
46+
if (changedLines <= SMALL_PR_LINES && files.length <= SMALL_PR_FILES) {
47+
messages.push('Thanks for keeping this pull request small.');
48+
}
49+
if (changedLines > SMALL_PR_LINES) warnings.push(`This PR changes more than ${SMALL_PR_LINES} lines.`);
50+
if (files.length > SMALL_PR_FILES) warnings.push(`This PR changes more than ${SMALL_PR_FILES} files.`);
51+
52+
if (files.some((file) => isAddedOrModified(file) && /test.*\.[jt]sx?$/.test(file.filename))) {
53+
messages.push('Thanks for updating tests! Only you can prevent production fires.');
54+
}
55+
if (files.some((file) => isAddedOrModified(file) && /(^|\/).*\.md$/.test(file.filename))) {
56+
messages.push('Thanks for updating documentation.');
57+
}
58+
if (files.some((file) => file.status === 'modified' && /(^|\/)package\.json$/.test(file.filename))) {
59+
warnings.push('package.json changed. Ensure applicable lock files are updated.');
60+
}
61+
62+
return { failures, warnings, messages, additions, deletions, changedLines, changedFiles: files.length };
63+
}
64+
65+
async function fetchChangedFiles(repository, pullNumber, token) {
66+
const files = [];
67+
for (let page = 1; ; page += 1) {
68+
const response = await fetch(
69+
`https://api.github.com/repos/${repository}/pulls/${pullNumber}/files?per_page=100&page=${page}`,
70+
{
71+
headers: {
72+
Accept: 'application/vnd.github+json',
73+
Authorization: `Bearer ${token}`,
74+
'X-GitHub-Api-Version': '2022-11-28',
75+
},
76+
},
77+
);
78+
if (!response.ok) throw new Error(`GitHub returned HTTP ${response.status} while listing pull request files.`);
79+
const pageFiles = await response.json();
80+
files.push(...pageFiles);
81+
if (pageFiles.length < 100) return files;
82+
}
83+
}
84+
85+
function renderSummary(review) {
86+
const sections = [
87+
'# Pull request review',
88+
`Files: ${review.changedFiles} · Lines added: ${review.additions} · Lines removed: ${review.deletions}`,
89+
];
90+
if (review.failures.length) sections.push('## Required fixes', ...review.failures.map((item) => `- ${item}`));
91+
if (review.warnings.length) sections.push('## Warnings', ...review.warnings.map((item) => `- ${item}`));
92+
if (review.messages.length) sections.push('## Notes', ...review.messages.map((item) => `- ${item}`));
93+
return `${sections.join('\n')}\n`;
94+
}
95+
96+
async function main() {
97+
const event = JSON.parse(readFileSync(process.env.GITHUB_EVENT_PATH, 'utf8'));
98+
const pullRequest = event.pull_request;
99+
if (!pullRequest?.number) throw new Error('This workflow requires a pull_request event.');
100+
101+
const files = await fetchChangedFiles(process.env.GITHUB_REPOSITORY, pullRequest.number, process.env.GITHUB_TOKEN);
102+
const review = reviewPullRequest(pullRequest, files);
103+
const summary = renderSummary(review);
104+
if (process.env.GITHUB_STEP_SUMMARY) appendFileSync(process.env.GITHUB_STEP_SUMMARY, summary);
105+
process.stdout.write(summary);
106+
for (const failure of review.failures) process.stdout.write(`::error::${failure}\n`);
107+
for (const warning of review.warnings) process.stdout.write(`::warning::${warning}\n`);
108+
for (const message of review.messages) process.stdout.write(`::notice::${message}\n`);
109+
if (review.failures.length) process.exitCode = 1;
110+
}
111+
112+
if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
113+
main().catch((error) => {
114+
process.stderr.write(`${error.message}\n`);
115+
process.exitCode = 1;
116+
});
117+
}

‎.github/scripts/pr-review.test.mjs‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import assert from 'node:assert/strict';
2+
import test from 'node:test';
3+
import { reviewPullRequest } from './pr-review.mjs';
4+
5+
const checklist = [
6+
'My code follows the style guidelines of this project',
7+
'I have performed a self-review of the code',
8+
'I have commented my code, particularly in hard-to-understand areas',
9+
'I have made corresponding changes to the documentation',
10+
'My changes generate no new warnings',
11+
'Any dependent changes have been merged and published in downstream modules',
12+
'I have checked my code and corrected any misspellings',
13+
];
14+
const validPullRequest = {
15+
title: 'docs: clarify installation',
16+
body: [
17+
'## Description',
18+
'Clarify setup instructions.',
19+
'## Type of Change',
20+
'Documentation',
21+
'## How Has This Been Tested?',
22+
'Reviewed the generated page.',
23+
'## Checklist',
24+
...checklist.map((item) => `- [x] ${item}`),
25+
].join('\n'),
26+
};
27+
28+
test('accepts a complete small pull request', () => {
29+
const result = reviewPullRequest(validPullRequest, [
30+
{ filename: 'src/app.test.tsx', status: 'modified', additions: 2, deletions: 1 },
31+
{ filename: 'docs/guide.md', status: 'modified', additions: 5, deletions: 0 },
32+
]);
33+
34+
assert.deepEqual(result.failures, []);
35+
assert.deepEqual(result.warnings, []);
36+
assert.ok(result.messages.includes('Thanks for updating tests! Only you can prevent production fires.'));
37+
assert.ok(result.messages.includes('Thanks for updating documentation.'));
38+
});
39+
40+
test('fails missing title, description, and required sections and warns on unchecked items', () => {
41+
const result = reviewPullRequest({ title: '', body: '' }, []);
42+
43+
assert.equal(result.failures.length, 6);
44+
assert.equal(result.warnings.length, checklist.length);
45+
});
46+
47+
test('warns when the diff exceeds either threshold', () => {
48+
const files = Array.from({ length: 11 }, (_, index) => ({
49+
filename: `src/file-${index}.tsx`,
50+
status: 'modified',
51+
additions: 20,
52+
deletions: 0,
53+
}));
54+
const result = reviewPullRequest(validPullRequest, files);
55+
56+
assert.ok(result.warnings.includes('This PR changes more than 200 lines.'));
57+
assert.ok(result.warnings.includes('This PR changes more than 10 files.'));
58+
});
59+
60+
test('warns when a package manifest changes', () => {
61+
const result = reviewPullRequest(validPullRequest, [
62+
{ filename: 'package.json', status: 'modified', additions: 1, deletions: 1 },
63+
]);
64+
65+
assert.ok(result.warnings.includes('package.json changed. Ensure applicable lock files are updated.'));
66+
});

‎.github/workflows/pr-review.yml‎

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,41 +10,32 @@ on:
1010
branches:
1111
- main
1212

13-
permissions:
14-
actions: write
15-
checks: write
16-
contents: write
17-
issues: write
18-
pull-requests: write
19-
statuses: write
20-
2113
jobs:
2214
pr-review:
15+
name: PR Review
2316
if: github.event_name == 'pull_request' && github.event.pull_request.draft == false
2417

2518
runs-on: ubuntu-latest
2619

20+
permissions:
21+
contents: read
22+
pull-requests: read
23+
2724
steps:
2825
- name: Checkout
2926
uses: actions/checkout@v7
3027

31-
- name: Setup Danger Files
32-
run: |
33-
echo "Setting up Danger files..."
34-
mv tools/danger/* .
35-
3628
- uses: actions/setup-node@v7
3729
with:
3830
node-version-file: .node-version
3931

40-
- name: Install dependencies
41-
run: npm install
32+
- name: Test PR review rules
33+
run: node --test .github/scripts/pr-review.test.mjs
4234

43-
- name: Danger JS Action
44-
# Pinned to main: tag 14.0.6 ships FROM node:20-slim, whose transitive
45-
# @yao-pkg/pkg requires node >=22 (docker build fails). Upstream main
46-
# already uses FROM node:22-slim; revert to a tag once released.
47-
uses: danger/danger-js@main
35+
- name: Validate pull request
36+
run: node .github/scripts/pr-review.mjs
4837
env:
49-
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
50-
DANGER_GITHUB_API_TOKEN: ${{ secrets.DANGER_GITHUB_API_TOKEN }}
38+
GITHUB_TOKEN: ${{ github.token }}
39+
GITHUB_REPOSITORY: ${{ github.repository }}
40+
GITHUB_EVENT_PATH: ${{ github.event_path }}
41+
GITHUB_STEP_SUMMARY: ${{ github.step_summary }}

‎tools/danger/.gitignore‎

Lines changed: 0 additions & 84 deletions
This file was deleted.

‎tools/danger/.node-version‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

0 commit comments

Comments
 (0)