From 5852d1ebb3170445b8675841fe50b54f05c1bc9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Wed, 16 Mar 2022 12:45:00 -0600 Subject: [PATCH 1/2] update PR checklist --- .github/PULL_REQUEST_TEMPLATE.md | 70 +++++++++++++++++++++----------- 1 file changed, 46 insertions(+), 24 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 6a8803999560..fd9a4c186db3 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -40,33 +40,44 @@ This is a checklist for PR authors & reviewers. Please make sure to complete all - [ ] I wrote clear testing steps that cover the changes made in this PR - [ ] I added steps for local testing in the `Tests` section - [ ] I added steps for Staging and/or Production testing in the `QA steps` section - - [ ] I added steps to cover failure scenarios (if applicable, i.e. verify an input displays the correct error message if the entered data is not correct) - - [ ] I added steps to cover offline scenarios (if applicable, i.e. verify the default avatar icon is displayed if app is offline) + - [ ] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct) + - [ ] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline) - [ ] I included screenshots or videos for tests on [all platforms](https://github.com/Expensify/App/blob/main/CONTRIBUTING.md#make-sure-you-can-test-on-all-platforms) -- [ ] I ran the tests & verified they passed on: +- [ ] I ran the tests on **all platforms** & verified they passed on: - [ ] iOS / native - [ ] Android / native - [ ] iOS / Safari - [ ] Android / Chrome - [ ] MacOS / Chrome - [ ] MacOS / Desktop -- [ ] I verified there are no console errors related to changes in this PR +- [ ] 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) - [ ] I followed proper code patterns (see [Reviewing the code](https://github.com/Expensify/App/blob/main/PR_REVIEW_GUIDELINES.md#reviewing-the-code)) - - [ ] I added comments when the code was not self explanatory - - [ ] I put all copy / text shown in the product in all `src/languages/*` files (if applicable) - - [ ] I followed proper naming convention for platform-specific files (if applicable) - - [ ] I followed style guidelines (in [`Styling.md`](https://github.com/Expensify/App/blob/main/STYLING.md)) for all style edits I made - - [ ] I followed the JSDocs style guidelines (in [`STYLE.md`](https://github.com/Expensify/App/blob/main/STYLE.md#jsdocs)) + - [ ] 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”) + - [ ] I verified that comments were added to code that is not self explanatory + - [ ] I verified that any new or modified comments were clear, correct english, and explained “why” the code was doing something instead of only explaining “what” the code was doing. + - [ ] I verified any copy / text shown in the product was added in all `src/languages/*` files + - [ ] I verified any copy / text that was added to the app is correct english and approved by marketing by tagging the marketing team on the original GH to get the correct copy. + - [ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named “index.js”. All platform-specific files are named for the platform the code supports as outlined in the README. + - [ ] I verified the JSDocs style guidelines (in [`STYLE.md`](https://github.com/Expensify/App/blob/main/STYLE.md#jsdocs)) were followed +- [ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers - [ ] I followed the guidelines as stated in the [Review Guidelines](https://github.com/Expensify/App/blob/main/PR_REVIEW_GUIDELINES.md) - [ ] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like `Avatar`, I verified the components using `Avatar` are working as expected) +- [ ] I verified all code is DRY +- [ ] I verified any variables that can be defined as constants (ie. in CONST.js) are defined as such - [ ] If a new component is created I verified that: - [ ] A similar component doesn't exist in the codebase - - [ ] Has the `displayName` property if it’s a Functional Component - - [ ] Has Storybook stories (optional) - - [ ] Has Unit tests (optional) + - [ ] All props are defined accurately and each prop has a `/** comment above it */` + - [ ] Any functional components have the `displayName` property + - [ ] The file is named correctly + - [ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone + - [ ] The only data being stored in the state is data necessary for rendering and nothing else + - [ ] Any internal methods are bound to “this” properly so there are no scoping issues + - [ ] Any internal methods bound to “this” are necessary to be bound + - [ ] All JSX used for rendering exists in the render method + - [ ] The component has the minimum amount of code necessary for its purpose and it is - [ ] If a new CSS style is added I verified that: - [ ] A similar style doesn’t already exist - - [ ] The style can’t be created with a [StyleUtils](https://github.com/Expensify/App/blob/main/src/styles/StyleUtils.js) function + - [ ] The style can’t be created with an existing [StyleUtils](https://github.com/Expensify/App/blob/main/src/styles/StyleUtils.js) function (i.e. `StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG`) @@ -76,8 +87,8 @@ This is a checklist for PR authors & reviewers. Please make sure to complete all - [ ] I verified testing steps are clear and they cover the changes made in this PR - [ ] I verified the steps for local testing are in the `Tests` section - [ ] I verified the steps for Staging and/or Production testing are in the `QA steps` section - - [ ] I verified the steps cover failure scenarios (if applicable, i.e. verify an input displays the correct error message if the entered data is not correct) - - [ ] I verified steps cover offline scenarios (if applicable, i.e. verify the default avatar icon is displayed if app is offline) + - [ ] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct) + - [ ] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline) - [ ] I checked that screenshots or videos are included for tests on [all platforms](https://github.com/Expensify/App/blob/main/CONTRIBUTING.md#make-sure-you-can-test-on-all-platforms) - [ ] I verified tests pass on **all platforms** & I tested again on: - [ ] iOS / native @@ -86,23 +97,34 @@ This is a checklist for PR authors & reviewers. Please make sure to complete all - [ ] Android / Chrome - [ ] MacOS / Chrome - [ ] MacOS / Desktop -- [ ] I verified there are no console errors related to changes in this PR +- [ ] 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) - [ ] I verified proper code patterns were followed (see [Reviewing the code](https://github.com/Expensify/App/blob/main/PR_REVIEW_GUIDELINES.md#reviewing-the-code)) - - [ ] I verified comments were added when the code was not self explanatory - - [ ] I verified any copy / text shown in the product was added in all `src/languages/*` files (if applicable) - - [ ] I verified proper naming convention for platform-specific files was followed (if applicable) - - [ ] I verified [style guidelines](https://github.com/Expensify/App/blob/main/STYLING.md) were followed + - [ ] 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”). + - [ ] I verified that comments were added to code that is not self explanatory + - [ ] I verified that any new or modified comments were clear, correct english, and explained “why” the code was doing something instead of only explaining “what” the code was doing. + - [ ] I verified any copy / text shown in the product was added in all `src/languages/*` files + - [ ] I verified any copy / text that was added to the app is correct english and approved by marketing by tagging the marketing team on the original GH to get the correct copy. + - [ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named “index.js”. All platform-specific files are named for the platform the code supports as outlined in the README. - [ ] I verified the JSDocs style guidelines (in [`STYLE.md`](https://github.com/Expensify/App/blob/main/STYLE.md#jsdocs)) were followed +- [ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers - [ ] I verified that this PR follows the guidelines as stated in the [Review Guidelines](https://github.com/Expensify/App/blob/main/PR_REVIEW_GUIDELINES.md) - [ ] I verified other components are not impacted by changes in this PR (i.e. if the PR modifies a shared library or component like `Avatar`, I verified the components using `Avatar` are working as expected) +- [ ] I verified all code is DRY +- [ ] I verified any variables that can be defined as constants (ie. in CONST.js) are defined as such - [ ] If a new component is created I verified that: - [ ] A similar component doesn't exist in the codebase - - [ ] Has the `displayName` property if it’s a Functional Component - - [ ] Has Storybook stories (optional) - - [ ] Has Unit tests (optional) + - [ ] All props are defined accurately and each prop has a `/** comment above it */` + - [ ] Any functional components have the `displayName` property + - [ ] The file is named correctly + - [ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone + - [ ] The only data being stored in the state is data necessary for rendering and nothing else + - [ ] Any internal methods are bound to “this” properly so there are no scoping issues + - [ ] Any internal methods bound to “this” are necessary to be bound + - [ ] All JSX used for rendering exists in the render method + - [ ] The component has the minimum amount of code necessary for its purpose and it is broken down into smaller components in order to separate concerns and functions - [ ] If a new CSS style is added I verified that: - [ ] A similar style doesn’t already exist - - [ ] The style can’t be created with a [StyleUtils](https://github.com/Expensify/App/blob/main/src/styles/StyleUtils.js) function + - [ ] The style can’t be created with an existing [StyleUtils](https://github.com/Expensify/App/blob/main/src/styles/StyleUtils.js) function (i.e. `StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG`) ### QA Steps From 4b54cb340676211bbdf8032f14f17bb302d4a550 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Wed, 16 Mar 2022 14:03:57 -0600 Subject: [PATCH 2/2] fix extra space --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index fd9a4c186db3..1d117e7c366a 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -52,7 +52,7 @@ This is a checklist for PR authors & reviewers. Please make sure to complete all - [ ] MacOS / Desktop - [ ] 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) - [ ] I followed proper code patterns (see [Reviewing the code](https://github.com/Expensify/App/blob/main/PR_REVIEW_GUIDELINES.md#reviewing-the-code)) - - [ ] 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”) + - [ ] 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”) - [ ] I verified that comments were added to code that is not self explanatory - [ ] I verified that any new or modified comments were clear, correct english, and explained “why” the code was doing something instead of only explaining “what” the code was doing. - [ ] I verified any copy / text shown in the product was added in all `src/languages/*` files