Skip to content

Commit 3ce9d50

Browse files
MrMuzykMrMuzyk
committed
feat: improve checklist script, update PR template and add verify script
1 parent b985182 commit 3ce9d50

5 files changed

Lines changed: 233 additions & 78 deletions

File tree

‎.github/PULL_REQUEST_TEMPLATE.md‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ Each set of changes should be tested against the Expensify/App repo on all platf
3030
- [ ] iOS / native
3131
- [ ] iOS / Safari
3232
- [ ] MacOS / Chrome / Safari
33-
- [ ] MacOS / Desktop
3433
- [ ] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
3534
- [ ] I followed proper code patterns (see [Reviewing the code](https://github.com/Expensify/App/blob/main/contributingGuides/PR_REVIEW_GUIDELINES.md#reviewing-the-code))
3635
- [ ] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. `toggleReport` and not `onIconClick`)
@@ -98,9 +97,3 @@ Each set of changes should be tested against the Expensify/App repo on all platf
9897

9998
</details>
10099

101-
<details>
102-
<summary>MacOS: Desktop</summary>
103-
104-
<!-- add screenshots or videos here -->
105-
106-
</details>

‎.github/actions/javascript/validatePRChecklist/index.js‎

Lines changed: 82 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -29979,49 +29979,98 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
2997929979
Object.defineProperty(exports, "__esModule", ({ value: true }));
2998029980
const core = __importStar(__nccwpck_require__(2186));
2998129981
const github = __importStar(__nccwpck_require__(5438));
29982+
const fs = __importStar(__nccwpck_require__(7147));
29983+
const path = __importStar(__nccwpck_require__(1017));
29984+
const UNCHECKED_PATTERN = /- \[ \]/g;
29985+
const CHECKED_PATTERN = /- \[x\]/gi;
29986+
function getAuthorChecklistSection(body) {
29987+
const startMarker = '### Author Checklist';
29988+
const endMarker = '### Screenshots/Videos';
29989+
const startIndex = body.indexOf(startMarker);
29990+
if (startIndex === -1) {
29991+
return '';
29992+
}
29993+
const afterStart = startIndex + startMarker.length;
29994+
const endIndex = body.indexOf(endMarker, afterStart);
29995+
return endIndex === -1 ? body.substring(afterStart) : body.substring(afterStart, endIndex);
29996+
}
29997+
function getExpectedChecklistCount() {
29998+
const templatePath = path.resolve(process.env.GITHUB_WORKSPACE || '.', '.github/PULL_REQUEST_TEMPLATE.md');
29999+
const template = fs.readFileSync(templatePath, 'utf8');
30000+
const checklistSection = getAuthorChecklistSection(template);
30001+
return (checklistSection.match(/- \[ \]/g) || []).length;
30002+
}
2998230003
function getSectionContent(body, sectionName) {
2998330004
const match = body.match(new RegExp(`### ${sectionName}\\s*\\n([\\s\\S]*?)(?=###|$)`));
2998430005
return ((match === null || match === void 0 ? void 0 : match[1]) || '').replace(/<!-[\s\S]*?->/g, '').trim();
2998530006
}
30007+
function validateChecklist(body) {
30008+
const errors = [];
30009+
const warnings = [];
30010+
const checklistSection = getAuthorChecklistSection(body);
30011+
if (!checklistSection.trim()) {
30012+
return {
30013+
errors: ['Author Checklist section not found. Please use the PR template.'],
30014+
warnings: [],
30015+
stats: { expected: 0, found: 0, checked: 0, unchecked: 0 },
30016+
};
30017+
}
30018+
const unchecked = (checklistSection.match(UNCHECKED_PATTERN) || []).length;
30019+
const checked = (checklistSection.match(CHECKED_PATTERN) || []).length;
30020+
const found = unchecked + checked;
30021+
const expected = getExpectedChecklistCount();
30022+
if (found < expected) {
30023+
errors.push(`Found ${found} checklist item(s) but expected at least ${expected}. It looks like items may have been removed. Please use the full PR template.`);
30024+
}
30025+
if (unchecked > 0) {
30026+
errors.push(`${unchecked} checklist item(s) are unchecked. All items must be checked before merging — including items that don't apply (check them and note why if needed).`);
30027+
}
30028+
// Section warnings
30029+
if (!getSectionContent(body, 'Automated Tests')) {
30030+
warnings.push('The "Automated Tests" section is empty. Please describe the automated tests you added, or explain why automated tests are not needed for this change.');
30031+
}
30032+
if (!getSectionContent(body, 'Manual Tests')) {
30033+
warnings.push('The "Manual Tests" section is empty. Please describe how you manually tested this change.');
30034+
}
30035+
const issues = getSectionContent(body, 'Related Issues');
30036+
if (issues === 'GH_LINK' || !issues) {
30037+
warnings.push('The "Related Issues" section still contains the GH_LINK placeholder or is empty. Please replace it with the actual GitHub issue link.');
30038+
}
30039+
return { errors, warnings, stats: { expected, found, checked, unchecked } };
30040+
}
30041+
function buildSummary(result) {
30042+
const parts = [];
30043+
if (result.errors.length > 0) {
30044+
parts.push('## PR Checklist Validation Failed\n');
30045+
parts.push('### Errors\n');
30046+
result.errors.forEach((e) => parts.push(`- ${e}`));
30047+
}
30048+
if (result.warnings.length > 0) {
30049+
parts.push('\n### Warnings\n');
30050+
result.warnings.forEach((w) => parts.push(`- ${w}`));
30051+
}
30052+
parts.push('\n### Checklist Stats\n');
30053+
parts.push('| Metric | Value |');
30054+
parts.push('|--------|-------|');
30055+
parts.push(`| Expected items | ${result.stats.expected} |`);
30056+
parts.push(`| Found items | ${result.stats.found} |`);
30057+
parts.push(`| Checked | ${result.stats.checked} |`);
30058+
parts.push(`| Unchecked | ${result.stats.unchecked} |`);
30059+
return parts.join('\n');
30060+
}
2998630061
function run() {
2998730062
return __awaiter(this, void 0, void 0, function* () {
2998830063
var _a;
2998930064
try {
2999030065
const body = ((_a = github.context.payload.pull_request) === null || _a === void 0 ? void 0 : _a.body) || '';
29991-
const errors = [];
29992-
// Check that all checklist items are checked
29993-
const uncheckedPattern = /- \[ \]/g;
29994-
const checkedPattern = /- \[x\]/gi;
29995-
const uncheckedMatches = body.match(uncheckedPattern) || [];
29996-
const checkedMatches = body.match(checkedPattern) || [];
29997-
const totalCheckboxes = uncheckedMatches.length + checkedMatches.length;
29998-
if (totalCheckboxes === 0) {
29999-
errors.push('No checklist items found in the PR description. Please use the PR template and complete the Author Checklist.');
30000-
}
30001-
else if (uncheckedMatches.length > 0) {
30002-
errors.push(`${uncheckedMatches.length} checklist item(s) are unchecked. All checklist items must be checked before merging — including items that don't apply (check them and note why if needed).`);
30003-
}
30004-
// Warn if "Automated Tests" section is empty
30005-
const automatedTestsContent = getSectionContent(body, 'Automated Tests');
30006-
if (automatedTestsContent.length === 0) {
30007-
core.warning('The "Automated Tests" section is empty. Please describe the automated tests you added, or explain why automated tests are not needed for this change.');
30008-
}
30009-
// Warn if "Manual Tests" section is empty
30010-
const manualTestsContent = getSectionContent(body, 'Manual Tests');
30011-
if (manualTestsContent.length === 0) {
30012-
core.warning('The "Manual Tests" section is empty. Please describe how you manually tested this change.');
30013-
}
30014-
// Warn if GH_LINK placeholder is still present
30015-
const relatedIssuesContent = getSectionContent(body, 'Related Issues');
30016-
if (relatedIssuesContent === 'GH_LINK' || relatedIssuesContent.length === 0) {
30017-
core.warning('The "Related Issues" section still contains the GH_LINK placeholder or is empty. Please replace it with the actual GitHub issue link.');
30018-
}
30019-
// Fail if there are errors
30020-
if (errors.length > 0) {
30021-
const summary = `## PR Checklist Validation Failed\n\n${errors.map((error) => `- ${error}`).join('\n')}\n\nPlease complete the checklist and update the PR description.`;
30022-
core.summary.addRaw(summary);
30066+
const result = validateChecklist(body);
30067+
result.warnings.forEach((w) => core.warning(w));
30068+
if (result.errors.length > 0 || result.warnings.length > 0) {
30069+
core.summary.addRaw(buildSummary(result));
3002330070
yield core.summary.write();
30024-
core.setFailed(errors.join('\n'));
30071+
}
30072+
if (result.errors.length > 0) {
30073+
core.setFailed(result.errors.join('\n'));
3002530074
}
3002630075
}
3002730076
catch (error) {

‎.github/actions/javascript/validatePRChecklist/validatePRChecklist.ts‎

Lines changed: 101 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -5,57 +5,120 @@
55

66
import * as core from '@actions/core';
77
import * as github from '@actions/github';
8+
import * as fs from 'fs';
9+
import * as path from 'path';
10+
11+
interface ValidationResult {
12+
errors: string[];
13+
warnings: string[];
14+
stats: {expected: number; found: number; checked: number; unchecked: number};
15+
}
16+
17+
const UNCHECKED_PATTERN = /- \[ \]/g;
18+
const CHECKED_PATTERN = /- \[x\]/gi;
19+
20+
function getAuthorChecklistSection(body: string): string {
21+
const startMarker = '### Author Checklist';
22+
const endMarker = '### Screenshots/Videos';
23+
const startIndex = body.indexOf(startMarker);
24+
if (startIndex === -1) {
25+
return '';
26+
}
27+
const afterStart = startIndex + startMarker.length;
28+
const endIndex = body.indexOf(endMarker, afterStart);
29+
return endIndex === -1 ? body.substring(afterStart) : body.substring(afterStart, endIndex);
30+
}
31+
32+
function getExpectedChecklistCount(): number {
33+
const templatePath = path.resolve(process.env.GITHUB_WORKSPACE || '.', '.github/PULL_REQUEST_TEMPLATE.md');
34+
const template = fs.readFileSync(templatePath, 'utf8');
35+
const checklistSection = getAuthorChecklistSection(template);
36+
return (checklistSection.match(/- \[ \]/g) || []).length;
37+
}
838

939
function getSectionContent(body: string, sectionName: string): string {
1040
const match = body.match(new RegExp(`### ${sectionName}\\s*\\n([\\s\\S]*?)(?=###|$)`));
1141
return (match?.[1] || '').replace(/<!-[\s\S]*?->/g, '').trim();
1242
}
1343

44+
function validateChecklist(body: string): ValidationResult {
45+
const errors: string[] = [];
46+
const warnings: string[] = [];
47+
const checklistSection = getAuthorChecklistSection(body);
48+
49+
if (!checklistSection.trim()) {
50+
return {
51+
errors: ['Author Checklist section not found. Please use the PR template.'],
52+
warnings: [],
53+
stats: {expected: 0, found: 0, checked: 0, unchecked: 0},
54+
};
55+
}
56+
57+
const unchecked = (checklistSection.match(UNCHECKED_PATTERN) || []).length;
58+
const checked = (checklistSection.match(CHECKED_PATTERN) || []).length;
59+
const found = unchecked + checked;
60+
const expected = getExpectedChecklistCount();
61+
62+
if (found < expected) {
63+
errors.push(`Found ${found} checklist item(s) but expected at least ${expected}. It looks like items may have been removed. Please use the full PR template.`);
64+
}
65+
66+
if (unchecked > 0) {
67+
errors.push(
68+
`${unchecked} checklist item(s) are unchecked. All items must be checked before merging — including items that don't apply (check them and note why if needed).`,
69+
);
70+
}
71+
72+
// Section warnings
73+
if (!getSectionContent(body, 'Automated Tests')) {
74+
warnings.push('The "Automated Tests" section is empty. Please describe the automated tests you added, or explain why automated tests are not needed for this change.');
75+
}
76+
if (!getSectionContent(body, 'Manual Tests')) {
77+
warnings.push('The "Manual Tests" section is empty. Please describe how you manually tested this change.');
78+
}
79+
const issues = getSectionContent(body, 'Related Issues');
80+
if (issues === 'GH_LINK' || !issues) {
81+
warnings.push('The "Related Issues" section still contains the GH_LINK placeholder or is empty. Please replace it with the actual GitHub issue link.');
82+
}
83+
84+
return {errors, warnings, stats: {expected, found, checked, unchecked}};
85+
}
86+
87+
function buildSummary(result: ValidationResult): string {
88+
const parts: string[] = [];
89+
if (result.errors.length > 0) {
90+
parts.push('## PR Checklist Validation Failed\n');
91+
parts.push('### Errors\n');
92+
result.errors.forEach((e) => parts.push(`- ${e}`));
93+
}
94+
if (result.warnings.length > 0) {
95+
parts.push('\n### Warnings\n');
96+
result.warnings.forEach((w) => parts.push(`- ${w}`));
97+
}
98+
parts.push('\n### Checklist Stats\n');
99+
parts.push('| Metric | Value |');
100+
parts.push('|--------|-------|');
101+
parts.push(`| Expected items | ${result.stats.expected} |`);
102+
parts.push(`| Found items | ${result.stats.found} |`);
103+
parts.push(`| Checked | ${result.stats.checked} |`);
104+
parts.push(`| Unchecked | ${result.stats.unchecked} |`);
105+
return parts.join('\n');
106+
}
107+
14108
async function run() {
15109
try {
16110
const body = github.context.payload.pull_request?.body || '';
17-
const errors: string[] = [];
18-
19-
// Check that all checklist items are checked
20-
const uncheckedPattern = /- \[ \]/g;
21-
const checkedPattern = /- \[x\]/gi;
22-
const uncheckedMatches = body.match(uncheckedPattern) || [];
23-
const checkedMatches = body.match(checkedPattern) || [];
24-
const totalCheckboxes = uncheckedMatches.length + checkedMatches.length;
25-
26-
if (totalCheckboxes === 0) {
27-
errors.push('No checklist items found in the PR description. Please use the PR template and complete the Author Checklist.');
28-
} else if (uncheckedMatches.length > 0) {
29-
errors.push(
30-
`${uncheckedMatches.length} checklist item(s) are unchecked. All checklist items must be checked before merging — including items that don't apply (check them and note why if needed).`,
31-
);
32-
}
33-
34-
// Warn if "Automated Tests" section is empty
35-
const automatedTestsContent = getSectionContent(body, 'Automated Tests');
36-
if (automatedTestsContent.length === 0) {
37-
core.warning('The "Automated Tests" section is empty. Please describe the automated tests you added, or explain why automated tests are not needed for this change.');
38-
}
111+
const result = validateChecklist(body);
39112

40-
// Warn if "Manual Tests" section is empty
41-
const manualTestsContent = getSectionContent(body, 'Manual Tests');
42-
if (manualTestsContent.length === 0) {
43-
core.warning('The "Manual Tests" section is empty. Please describe how you manually tested this change.');
44-
}
113+
result.warnings.forEach((w) => core.warning(w));
45114

46-
// Warn if GH_LINK placeholder is still present
47-
const relatedIssuesContent = getSectionContent(body, 'Related Issues');
48-
if (relatedIssuesContent === 'GH_LINK' || relatedIssuesContent.length === 0) {
49-
core.warning('The "Related Issues" section still contains the GH_LINK placeholder or is empty. Please replace it with the actual GitHub issue link.');
115+
if (result.errors.length > 0 || result.warnings.length > 0) {
116+
core.summary.addRaw(buildSummary(result));
117+
await core.summary.write();
50118
}
51119

52-
// Fail if there are errors
53-
if (errors.length > 0) {
54-
const summary = `## PR Checklist Validation Failed\n\n${errors.map((error) => `- ${error}`).join('\n')}\n\nPlease complete the checklist and update the PR description.`;
55-
56-
core.summary.addRaw(summary);
57-
await core.summary.write();
58-
core.setFailed(errors.join('\n'));
120+
if (result.errors.length > 0) {
121+
core.setFailed(result.errors.join('\n'));
59122
}
60123
} catch (error) {
61124
core.setFailed((error as Error).message);

‎.github/scripts/verifyActions.sh‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
#!/bin/bash
2+
#
3+
# Re-compiles all Github Actions and verifies that there is no diff,
4+
# because that would indicate that the PR author forgot to run `npm run gh-actions-build`
5+
# and commit the re-bundled the javascript sources.
6+
7+
declare -r GREEN='\033[0;32m'
8+
declare -r RED='\033[0;31m'
9+
declare -r NC='\033[0m'
10+
11+
printf '\nRebuilding GitHub Actions...\n'
12+
npm run gh-actions-build
13+
14+
DIFF_OUTPUT=$(git diff --exit-code)
15+
EXIT_CODE=$?
16+
17+
if [[ EXIT_CODE -eq 0 ]]; then
18+
echo -e "${GREEN}Github Actions are up to date!${NC}"
19+
exit 0
20+
else
21+
echo -e "${RED}Error: Diff found when Github Actions were rebuilt. Did you forget to run \`npm run gh-actions-build\` after making changes?${NC}"
22+
echo "$DIFF_OUTPUT"
23+
exit 1
24+
fi

‎.github/workflows/validateGithubActions.yml‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
name: Validate Github Actions
2+
3+
on:
4+
pull_request:
5+
types: [opened, synchronize]
6+
paths: ['.github/**']
7+
8+
jobs:
9+
verify:
10+
runs-on: ubuntu-latest
11+
steps:
12+
- name: Checkout
13+
# v4.1.1
14+
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
15+
16+
- name: Setup Node
17+
uses: actions/setup-node@v4
18+
with:
19+
node-version-file: .nvmrc
20+
cache: npm
21+
22+
- name: Install dependencies
23+
run: npm ci
24+
25+
- name: Verify Javascript Action Builds
26+
run: ./.github/scripts/verifyActions.sh

0 commit comments

Comments
 (0)