Skip to content

Commit 5df5f4b

Browse files
authored
Merge pull request Expensify#790 from callstack-internal/VickyStash/refactor/91215-link-eApp-pr
[No QA] Require linked E/App PR in Onyx PR template
2 parents d674d7c + 1e06a26 commit 5df5f4b

3 files changed

Lines changed: 31 additions & 0 deletions

File tree

.github/PULL_REQUEST_TEMPLATE.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,18 @@
77
<!-- Please replace GH_LINK with the link to the GitHub issue this Pull Request is related to -->
88
GH_LINK
99

10+
### Linked E/App PR
11+
<!--
12+
Every Onyx PR must ship with a corresponding Expensify/App PR that pins Onyx to this PR's HEAD via git+https
13+
(e.g. "react-native-onyx": "git+https://github.com/Expensify/react-native-onyx.git#<commit_sha>").
14+
That linked PR runs the full E/App test suite signals that the Onyx
15+
change is safe. After this PR is merged and published to npm, swap the linked E/App PR to the pinned version
16+
(e.g. "react-native-onyx": "3.0.72") and request a final review — it becomes the bump PR.
17+
18+
Paste the full Expensify/App PR URL below (e.g. https://github.com/Expensify/App/pull/12345):
19+
-->
20+
21+
1022
### Automated Tests
1123
<!---
1224
Most changes to Onyx should have accompanying tests. Describe the tests you added or if no tests were added an explanation about why one was not needed.
@@ -20,6 +32,7 @@ Each set of changes should be tested against the Expensify/App repo on all platf
2032
### Author Checklist
2133

2234
- [ ] I linked the correct issue in the `### Related Issues` section above
35+
- [ ] I linked the corresponding Expensify/App PR in the `### Linked E/App PR` section above, and verified this change against it (E/App CI passed and manual testing completed)
2336
- [ ] I wrote clear testing steps that cover the changes made in this PR
2437
- [ ] I added steps for local testing in the `Tests` section
2538
- [ ] I tested this PR with a [High Traffic account](https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md#high-traffic-accounts) against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29983,6 +29983,7 @@ const fs = __importStar(__nccwpck_require__(7147));
2998329983
const path = __importStar(__nccwpck_require__(1017));
2998429984
const UNCHECKED_PATTERN = /- \[ \]/g;
2998529985
const CHECKED_PATTERN = /- \[x\]/gi;
29986+
const E_APP_PR_URL_PATTERN = /^https?:\/\/github\.com\/Expensify\/App\/pull\/\d+\/?$/;
2998629987
function getAuthorChecklistSection(body) {
2998729988
const startMarker = '### Author Checklist';
2998829989
const endMarker = '### Screenshots/Videos';
@@ -30025,6 +30026,13 @@ function validateChecklist(body) {
3002530026
if (unchecked > 0) {
3002630027
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).`);
3002730028
}
30029+
const linkedEAppPR = getSectionContent(body, 'Linked E/App PR');
30030+
if (!linkedEAppPR) {
30031+
errors.push('The "Linked E/App PR" section is empty. Every Onyx PR must link to a corresponding Expensify/App PR that pins this PR via git+https and runs the full E/App test suite.');
30032+
}
30033+
else if (!E_APP_PR_URL_PATTERN.test(linkedEAppPR)) {
30034+
errors.push(`The "Linked E/App PR" section must contain a single Expensify/App PR URL (e.g. https://github.com/Expensify/App/pull/12345), found: "${linkedEAppPR}".`);
30035+
}
3002830036
// Section warnings
3002930037
if (!getSectionContent(body, 'Automated Tests')) {
3003030038
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.');

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ interface ValidationResult {
1616

1717
const UNCHECKED_PATTERN = /- \[ \]/g;
1818
const CHECKED_PATTERN = /- \[x\]/gi;
19+
const E_APP_PR_URL_PATTERN = /^https?:\/\/github\.com\/Expensify\/App\/pull\/\d+\/?$/;
1920

2021
function getAuthorChecklistSection(body: string): string {
2122
const startMarker = '### Author Checklist';
@@ -67,6 +68,15 @@ function validateChecklist(body: string): ValidationResult {
6768
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).`);
6869
}
6970

71+
const linkedEAppPR = getSectionContent(body, 'Linked E/App PR');
72+
if (!linkedEAppPR) {
73+
errors.push(
74+
'The "Linked E/App PR" section is empty. Every Onyx PR must link to a corresponding Expensify/App PR that pins this PR via git+https and runs the full E/App test suite.',
75+
);
76+
} else if (!E_APP_PR_URL_PATTERN.test(linkedEAppPR)) {
77+
errors.push(`The "Linked E/App PR" section must contain a single Expensify/App PR URL (e.g. https://github.com/Expensify/App/pull/12345), found: "${linkedEAppPR}".`);
78+
}
79+
7080
// Section warnings
7181
if (!getSectionContent(body, 'Automated Tests')) {
7282
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.');

0 commit comments

Comments
 (0)