Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[No QA] rename checklist names to Author and Reviewer #11544

Merged
merged 7 commits into from
Oct 3, 2022
Merged

[No QA] rename checklist names to Author and Reviewer #11544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2141c82
rename contributor to reviewer
marcochavezf Oct 3, 2022
6178bd6
rename workflow names
marcochavezf Oct 3, 2022
2af3962
rename variables to use author and reviewer words
marcochavezf Oct 3, 2022
a51fd2d
change success and failed messages
marcochavezf Oct 3, 2022
d32e5f5
change variable value
marcochavezf Oct 3, 2022
0cc64e2
rebuilt new gh action
marcochavezf Oct 3, 2022
8f92b3a
re-use old CHECKLIST value
marcochavezf Oct 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/PULL_REQUEST_TEMPLATE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ For example:
<!--
This is a checklist for PR authors & reviewers. Please make sure to complete all tasks and check them off once you do, or else Expensify has the right not to merge your PR!
-->
#### Contributor (PR Author) Checklist
#### PR Author Checklist
- [ ] I linked the correct issue in the `### Fixed Issues` section above
- [ ] I wrote clear testing steps that cover the changes made in this PR
- [ ] I added steps for local testing in the `Tests` section
Expand Down Expand Up @@ -85,7 +85,7 @@ This is a checklist for PR authors & reviewers. Please make sure to complete all
<details>
<summary><h4>PR Reviewer Checklist</h4>

The Contributor+ will copy/paste it into a new comment and complete it after the author checklist is completed
The reviewer will copy/paste it into a new comment and complete it after the author checklist is completed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB, but I think this is ambiguous still as it implies that there is only one "reviewer". Some PRs have multiple reviewers. Who should do the checklist if no C+ is assigned?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the answer is that a C+ should always be assigned - but seems like there is not a total consensus on this and we haven't made it a formal process yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the C+ review, I think we can update (if required) the C+ doc to state that C+ should always have to fill out the PR reviewer checklist when they're assigned as reviewers.

</summary>

- [ ] I have verified the author checklist is complete (all boxes are checked off).
Expand Down
30 changes: 15 additions & 15 deletions .github/actions/javascript/contributorChecklist/contributorChecklist.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const _ = require('underscore');
const GitHubUtils = require('../../../libs/GithubUtils');

/* eslint-disable max-len */
const completedContributorChecklist = `- [x] I linked the correct issue in the \`### Fixed Issues\` section above
const completedAuthorChecklist = `- [x] I linked the correct issue in the \`### Fixed Issues\` section above
- [x] I wrote clear testing steps that cover the changes made in this PR
- [x] I added steps for local testing in the \`Tests\` section
- [x] I added steps for Staging and/or Production testing in the \`QA steps\` section
Expand Down Expand Up @@ -50,7 +50,7 @@ const completedContributorChecklist = `- [x] I linked the correct issue in the \
- [x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
- [x] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.`;

const completedContributorPlusChecklist = `- [x] I have verified the author checklist is complete (all boxes are checked off).
const completedReviewerChecklist = `- [x] I have verified the author checklist is complete (all boxes are checked off).
- [x] I verified the correct issue is linked in the \`### Fixed Issues\` section above
- [x] I verified testing steps are clear and they cover the changes made in this PR
- [x] I verified the steps for local testing are in the \`Tests\` section
Expand Down Expand Up @@ -97,8 +97,8 @@ const completedContributorPlusChecklist = `- [x] I have verified the author chec
- [x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
- [x] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.`;

// True if we are validating a contributor checklist, otherwise we are validating a contributor+ checklist
const verifyingContributorChecklist = core.getInput('CHECKLIST', {required: true}) === 'contributor';
// True if we are validating an author checklist, otherwise we are validating a reviewer checklist
const verifyingAuthorChecklist = core.getInput('CHECKLIST', {required: true}) === 'contributor';
const issue = github.context.payload.issue ? github.context.payload.issue.number : github.context.payload.pull_request.number;
const combinedData = [];

Expand Down Expand Up @@ -135,34 +135,34 @@ getPullRequestBody()
.then(() => getAllComments())
.then(comments => combinedData.push(...comments))
.then(() => {
let contributorChecklistComplete = false;
let contributorPlusChecklistComplete = false;
let authorChecklistComplete = false;
let reviewerChecklistComplete = false;

// Once we've gathered all the data, loop through each comment and look to see if it contains a completed checklist
for (let i = 0; i < combinedData.length; i++) {
const whitespace = /([\n\r])/gm;
const comment = combinedData[i].replace(whitespace, '');

if (comment.includes(completedContributorChecklist.replace(whitespace, ''))) {
contributorChecklistComplete = true;
if (comment.includes(completedAuthorChecklist.replace(whitespace, ''))) {
authorChecklistComplete = true;
}

if (comment.includes(completedContributorPlusChecklist.replace(whitespace, ''))) {
contributorPlusChecklistComplete = true;
if (comment.includes(completedReviewerChecklist.replace(whitespace, ''))) {
reviewerChecklistComplete = true;
}
}

if (verifyingContributorChecklist && !contributorChecklistComplete) {
if (verifyingAuthorChecklist && !authorChecklistComplete) {
console.log('Make sure you are using the most up to date checklist found here: https://raw.githubusercontent.com/Expensify/App/main/.github/PULL_REQUEST_TEMPLATE.md');
core.setFailed('Contributor checklist is not completely filled out. Please check every box to verify you\'ve thought about the item.');
core.setFailed('PR Author Checklist is not completely filled out. Please check every box to verify you\'ve thought about the item.');
return;
}

if (!verifyingContributorChecklist && !contributorPlusChecklistComplete) {
if (!verifyingAuthorChecklist && !reviewerChecklistComplete) {
console.log('Make sure you are using the most up to date checklist found here: https://raw.githubusercontent.com/Expensify/App/main/.github/PULL_REQUEST_TEMPLATE.md');
core.setFailed('Contributor+ checklist is not completely filled out. Please check every box to verify you\'ve thought about the item.');
core.setFailed('PR Reviewer Checklist is not completely filled out. Please check every box to verify you\'ve thought about the item.');
return;
}

console.log(`${verifyingContributorChecklist ? 'Contributor' : 'Contributor+'} checklist is complete 🎉`);
console.log(`${verifyingAuthorChecklist ? 'PR Author' : 'PR Reviewer'} checklist is complete 🎉`);
});
30 changes: 15 additions & 15 deletions .github/actions/javascript/contributorChecklist/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const _ = __nccwpck_require__(3571);
const GitHubUtils = __nccwpck_require__(7999);

/* eslint-disable max-len */
const completedContributorChecklist = `- [x] I linked the correct issue in the \`### Fixed Issues\` section above
const completedAuthorChecklist = `- [x] I linked the correct issue in the \`### Fixed Issues\` section above
- [x] I wrote clear testing steps that cover the changes made in this PR
- [x] I added steps for local testing in the \`Tests\` section
- [x] I added steps for Staging and/or Production testing in the \`QA steps\` section
Expand Down Expand Up @@ -60,7 +60,7 @@ const completedContributorChecklist = `- [x] I linked the correct issue in the \
- [x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
- [x] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.`;

const completedContributorPlusChecklist = `- [x] I have verified the author checklist is complete (all boxes are checked off).
const completedReviewerChecklist = `- [x] I have verified the author checklist is complete (all boxes are checked off).
- [x] I verified the correct issue is linked in the \`### Fixed Issues\` section above
- [x] I verified testing steps are clear and they cover the changes made in this PR
- [x] I verified the steps for local testing are in the \`Tests\` section
Expand Down Expand Up @@ -107,8 +107,8 @@ const completedContributorPlusChecklist = `- [x] I have verified the author chec
- [x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
- [x] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.`;

// True if we are validating a contributor checklist, otherwise we are validating a contributor+ checklist
const verifyingContributorChecklist = core.getInput('CHECKLIST', {required: true}) === 'contributor';
// True if we are validating an author checklist, otherwise we are validating a reviewer checklist
const verifyingAuthorChecklist = core.getInput('CHECKLIST', {required: true}) === 'contributor';
const issue = github.context.payload.issue ? github.context.payload.issue.number : github.context.payload.pull_request.number;
const combinedData = [];

Expand Down Expand Up @@ -145,36 +145,36 @@ getPullRequestBody()
.then(() => getAllComments())
.then(comments => combinedData.push(...comments))
.then(() => {
let contributorChecklistComplete = false;
let contributorPlusChecklistComplete = false;
let authorChecklistComplete = false;
let reviewerChecklistComplete = false;

// Once we've gathered all the data, loop through each comment and look to see if it contains a completed checklist
for (let i = 0; i < combinedData.length; i++) {
const whitespace = /([\n\r])/gm;
const comment = combinedData[i].replace(whitespace, '');

if (comment.includes(completedContributorChecklist.replace(whitespace, ''))) {
contributorChecklistComplete = true;
if (comment.includes(completedAuthorChecklist.replace(whitespace, ''))) {
authorChecklistComplete = true;
}

if (comment.includes(completedContributorPlusChecklist.replace(whitespace, ''))) {
contributorPlusChecklistComplete = true;
if (comment.includes(completedReviewerChecklist.replace(whitespace, ''))) {
reviewerChecklistComplete = true;
}
}

if (verifyingContributorChecklist && !contributorChecklistComplete) {
if (verifyingAuthorChecklist && !authorChecklistComplete) {
console.log('Make sure you are using the most up to date checklist found here: https://raw.githubusercontent.com/Expensify/App/main/.github/PULL_REQUEST_TEMPLATE.md');
core.setFailed('Contributor checklist is not completely filled out. Please check every box to verify you\'ve thought about the item.');
core.setFailed('PR Author Checklist is not completely filled out. Please check every box to verify you\'ve thought about the item.');
return;
}

if (!verifyingContributorChecklist && !contributorPlusChecklistComplete) {
if (!verifyingAuthorChecklist && !reviewerChecklistComplete) {
console.log('Make sure you are using the most up to date checklist found here: https://raw.githubusercontent.com/Expensify/App/main/.github/PULL_REQUEST_TEMPLATE.md');
core.setFailed('Contributor+ checklist is not completely filled out. Please check every box to verify you\'ve thought about the item.');
core.setFailed('PR Reviewer Checklist is not completely filled out. Please check every box to verify you\'ve thought about the item.');
return;
}

console.log(`${verifyingContributorChecklist ? 'Contributor' : 'Contributor+'} checklist is complete 🎉`);
console.log(`${verifyingAuthorChecklist ? 'PR Author' : 'PR Reviewer'} checklist is complete 🎉`);
});


Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/contributorChecklists.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Contributor Checklist
name: PR Author Checklist

on: pull_request

Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/contributorPlusChecklists.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Contributor+ Checklist
name: PR Reviewer Checklist

on: pull_request_review

Expand All @@ -11,4 +11,4 @@ jobs:
uses: Expensify/App/.github/actions/javascript/contributorChecklist@main
with:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
CHECKLIST: 'contributorPlus'
CHECKLIST: 'reviewer'