Update CONTRIBUTING.md

[mallenexpensify]

Line 96

7. Once hired, post a comment in the Github issue stating when you expect to have your PR ready for review

Details

Fixed Issues

$ GH_LINK
PROPOSAL: GH_LINK_ISSUE(COMMENT)

Tests

Open https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md to confirm line 96 is updated to read

7. Once hired, post a comment in the Github issue stating when you expect to have your PR ready for review

• Verify that no errors appear in the JS console

QA Steps

• Verify that no errors appear in the JS console

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
  • I added steps for Staging and/or Production testing in the QA steps section
  • 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
• 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 (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)
  • 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 adding the Waiting for Copy label for a copy review 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) 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
• 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 (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• If a new component is created I verified that:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately and each prop has a /** comment above it */
  • 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
  • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • 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 an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• 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.
• I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

PR Reviewer Checklist

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

• I have verified the author checklist is complete (all boxes are checked off).
• I verified the correct issue is linked in the ### Fixed Issues section above
• 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 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
• I included screenshots or videos for tests on all platforms
• I verified tests pass on all platforms & I tested again on:
  • iOS / native
  • Android / native
  • iOS / Safari
  • Android / Chrome
  • MacOS / Chrome
  • MacOS / Desktop
• If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
• I verified proper code patterns were followed (see 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 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 adding the Waiting for Copy label for a copy review 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) 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
• I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• If a new component is created I verified that:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately and each prop has a /** comment above it */
  • 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
  • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • 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 an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• 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.
• I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

[Justicea83]

Looks good but I can't see the issue this PR is fixing in your PR description.

[mallenexpensify]

@Justicea83 there's no GH issue cuz it's just a line being updated in CONTRIBUTING.md

[Justicea83]

Reviewer Checklist

• I have verified the author checklist is complete (all boxes are checked off).
• I verified the correct issue is linked in the ### Fixed Issues section above
• 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 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
• I included screenshots or videos for tests on all platforms
• I verified tests pass on all platforms & I tested again on:
  • iOS / native
  • Android / native
  • iOS / Safari
  • Android / Chrome
  • MacOS / Chrome
  • MacOS / Desktop
• If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
• I verified proper code patterns were followed (see 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 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 adding the Waiting for Copy label for a copy review 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) 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
• I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• If a new component is created I verified that:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately and each prop has a /** comment above it */
  • 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
  • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • 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 an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• 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.
• I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

[Justicea83]

LGTM!

[melvin-bot]

@jasperhuangg Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

[mallenexpensify]

@jasperhuangg , looks like I forgot about this then @Justicea83 left. Can you review plz?

[jasperhuangg]

Reviewer Checklist

• I have verified the author checklist is complete (all boxes are checked off).
• I verified the correct issue is linked in the ### Fixed Issues section above
• 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 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
• I included screenshots or videos for tests on all platforms
• I verified tests pass on all platforms & I tested again on:
  • iOS / native
  • Android / native
  • iOS / Safari
  • Android / Chrome
  • MacOS / Chrome
  • MacOS / Desktop
• If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
• I verified proper code patterns were followed (see 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 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 adding the Waiting for Copy label for a copy review 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) 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
• I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• If a new component is created I verified that:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately and each prop has a /** comment above it */
  • 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
  • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • 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 an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• 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.
• I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

[jasperhuangg]

This just updates a markdown file, and the PR reviewer and author checklists are both clearly completed, so I'm going to go ahead and merge.

[OSBotify]

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

[github-actions]

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
App start nativeLaunch	9.967 ms → 20.581 ms (+10.614 ms, +106.5%) 🔴
Open Search Page TTI	609.673 ms → 613.044 ms (+3.371 ms, +0.6%)
App start TTI	658.157 ms → 658.932 ms (+0.775 ms, ±0.0%)
App start regularAppStart	0.014 ms → 0.021 ms (+0.007 ms, +47.1%) 🔴
App start runJsBundle	182.438 ms → 181.000 ms (-1.438 ms, -0.8%)

Show details

Name	Duration
App start nativeLaunch	Baseline Mean: 9.967 ms Stdev: 1.449 ms (14.5%) Runs: 8 8 8 8 9 9 9 9 9 9 9 9 9 9 10 10 10 10 10 10 10 11 11 11 12 12 12 12 13 13 Current Mean: 20.581 ms Stdev: 2.167 ms (10.5%) Runs: 18 18 18 18 18 19 19 19 19 19 19 19 19 20 20 20 21 21 21 21 21 21 22 22 22 22 23 23 24 26 26
Open Search Page TTI	Baseline Mean: 609.673 ms Stdev: 19.366 ms (3.2%) Runs: 568.2221679985523 570.1571449991316 582.2211910001934 585.1941320002079 587.3348399996758 590.397624000907 594.1842040009797 596.2937020007521 596.9753019995987 599.3367929998785 600.8310150001198 602.9091799985617 604.8335370011628 606.0080969985574 607.3569750003517 610.3533939998597 610.5683999992907 611.015868999064 612.0985919982195 615.0898029990494 616.1014000009745 622.893636001274 623.2305100001395 624.2639160007238 626.3178310003132 627.6153979990631 628.2281499989331 634.8924560006708 635.5666910000145 636.8067630007863 637.1748869996518 645.0598959997296 Current Mean: 613.044 ms Stdev: 26.701 ms (4.4%) Runs: 551.8569740019739 578.2101639993489 578.2526040002704 579.8677979968488 581.6964519992471 584.4394129998982 586.1421310007572 588.9866949990392 590.492837999016 600.2827959991992 601.1755780018866 601.5701909996569 604.2991129979491 606.8773189969361 607.0606289990246 609.8396820016205 613.9729820005596 614.0151780024171 616.2135830000043 617.6917730011046 620.1379799991846 625.812947999686 629.7395429983735 631.7144780009985 632.6241050027311 632.765625 634.4556070007384 639.2274170033634 644.1640219986439 644.3574220016599 649.5053310021758 655.0096440017223 678.006389003247
App start TTI	Baseline Mean: 658.157 ms Stdev: 31.775 ms (4.8%) Runs: 577.6411649994552 616.049633000046 616.4365369994193 619.5047839991748 627.4864680003375 632.7729710005224 635.5671120006591 639.4337039999664 639.9998850002885 640.4828960001469 645.0767570007592 646.1174420006573 654.2104899995029 655.6369589995593 658.0675980001688 661.3788270000368 665.525761000812 666.7289409991354 668.4022610001266 669.5011519994587 672.8376630004495 674.3423050008714 678.7207360006869 680.5321430005133 683.0706469994038 683.5139770004898 693.3135429993272 694.8036599997431 699.8221080005169 747.7241149991751 Current Mean: 658.932 ms Stdev: 29.501 ms (4.5%) Runs: 596.7514360006899 621.8307280000299 624.9711949992925 626.8049660008401 630.0362339988351 630.0858299992979 633.674633000046 635.2506349999458 636.3944239988923 644.4439700003713 646.7230079993606 646.8655290007591 648.1743359994143 650.4862799998373 651.7367889992893 656.8811739999801 657.1395280007273 657.2957169990987 657.8588219992816 658.131441000849 666.992851998657 673.5637369994074 677.6026889998466 678.7476080004126 686.6199210006744 689.973995000124 691.9759330004454 704.2861099988222 711.3684049993753 715.6985909994692 718.5194269996136
App start regularAppStart	Baseline Mean: 0.014 ms Stdev: 0.001 ms (8.9%) Runs: 0.012044999748468399 0.012613000348210335 0.012614000588655472 0.012735998257994652 0.012775998562574387 0.013020999729633331 0.013061000034213066 0.013061998412013054 0.013062000274658203 0.013102000579237938 0.013508999720215797 0.01355000026524067 0.013589998707175255 0.013631999492645264 0.014038000255823135 0.014851998537778854 0.014932999387383461 0.014932999387383461 0.01517699845135212 0.015258999541401863 0.015300000086426735 0.015381000936031342 0.015421001240611076 0.015422001481056213 0.015584999695420265 0.015829000622034073 0.01619500108063221 0.016479000449180603 Current Mean: 0.021 ms Stdev: 0.001 ms (6.5%) Runs: 0.0186769999563694 0.019043000414967537 0.019082998856902122 0.019246000796556473 0.01924699917435646 0.019409000873565674 0.019530998542904854 0.019694000482559204 0.01977499946951866 0.019979000091552734 0.020142000168561935 0.020549001172184944 0.020670000463724136 0.020793000236153603 0.020832998678088188 0.020955000072717667 0.02103700116276741 0.021077999845147133 0.021158000454306602 0.021198999136686325 0.021402999758720398 0.0215659998357296 0.021687999367713928 0.021891999989748 0.02213599905371666 0.022257000207901 0.022704999893903732 0.022908002138137817 0.022990001365542412 0.024292001500725746
App start runJsBundle	Baseline Mean: 182.438 ms Stdev: 22.192 ms (12.2%) Runs: 151 153 156 157 157 159 162 164 166 166 169 173 174 177 179 180 181 183 183 183 187 187 192 194 198 198 205 207 210 213 228 246 Current Mean: 181.000 ms Stdev: 18.486 ms (10.2%) Runs: 154 159 161 164 164 167 167 167 167 170 170 172 172 173 174 177 177 179 181 183 184 186 187 188 192 198 201 207 216 227 227

[github-actions]

@Expensify/mobile-deployers 📣 Please look into this performance regression as it's a deploy blocker.

[OSBotify]

🚀 Deployed to staging by @jasperhuangg in version: 1.2.52-0 🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	failure ❌
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[OSBotify]

🚀 Deployed to production by @Julesssss in version: 1.2.52-4 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅