Update CONTRIBUTING.md

[dylanexpensify]

Pulling now, but will add link to best practices once done

Details

related to https://github.com/Expensify/Expensify/issues/256253, updating process

Fixed Issues

related https://github.com/Expensify/Expensify/issues/256253 - makes process doc up to date

Tests

• Verify that no errors appear in the JS console

Offline tests

N/A

QA Steps

N/A

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 the expected offline behavior in the Offline steps 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 tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • 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 is localized by adding it to src/languages/* files and using the translation method
    • If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • 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
• I verified that if a function's arguments changed that all usages have also been updated correctly
• 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 any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• 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.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

No screenshots required just readme update

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

[melvin-bot]

@sketchydroide 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]

[mountiny]

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:
  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • 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 is localized by adding it to src/languages/* files and using the translation method
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • 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 any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• 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.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.
  No screenshots required since this is just markdown file change

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

[mountiny]

Just couple suggestions

[mountiny]

@dylanexpensify just one more stylistic review, the other lines have dot at the end.

[dylanexpensify]

@mountiny accepted, great catch!

[mountiny]

Looking good, thanks!

[melvin-bot]

@mountiny looks like this was merged without a test passing. Please add a note explaining why this was done and remove the Emergency label if this is not an emergency.

[mountiny]

All checks have passed, liar melvin bot!

[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 TTI	683.933 ms → 706.391 ms (+22.458 ms, +3.3%)
App start runJsBundle	191.219 ms → 195.484 ms (+4.265 ms, +2.2%)
App start regularAppStart	0.014 ms → 0.015 ms (+0.001 ms, +8.1%)
App start nativeLaunch	20.406 ms → 20.097 ms (-0.309 ms, -1.5%)
Open Search Page TTI	604.926 ms → 604.296 ms (-0.631 ms, ±0.0%)

Show details

Name	Duration
App start TTI	Baseline Mean: 683.933 ms Stdev: 26.925 ms (3.9%) Runs: 635.9409309998155 645.736975999549 646.2130749998614 647.0228380002081 647.4006470004097 651.2238539997488 654.727811999619 659.4077960001305 669.3570680003613 670.4443910000846 671.5755129996687 676.4811049997807 676.9721499998122 678.6155470004305 682.752163999714 685.7365410001948 688.0996479997411 688.6920199999586 691.3920019995421 691.9142669998109 697.5031489999965 699.4994090003893 700.9732499998063 706.3127749999985 706.571055999957 707.4616489997134 708.9632329996675 717.4425200000405 723.9217579998076 735.2088810000569 738.3549750000238 Current Mean: 706.391 ms Stdev: 35.286 ms (5.0%) Runs: 640.9687209995463 652.8001870000735 660.1025130003691 665.1437470000237 666.0852910000831 673.7665280001238 677.6189139997587 687.2529309997335 688.6568409996107 690.2223629998043 690.4955080002546 690.8005339996889 691.4841590002179 692.1597619997337 693.6388199999928 694.8899529995397 699.6725209997967 701.6436259998009 703.875494999811 714.7421270003542 714.9784490000457 718.448827999644 723.0317209996283 729.0169789995998 730.3570950003341 732.1895439997315 746.2543569998816 751.405252000317 756.4461599998176 760.1929930001497 782.8355890000239 783.3415660001338
App start runJsBundle	Baseline Mean: 191.219 ms Stdev: 22.882 ms (12.0%) Runs: 158 160 162 163 168 172 173 173 175 178 178 179 180 180 182 185 187 188 191 194 197 199 204 206 211 212 215 215 223 225 230 256 Current Mean: 195.484 ms Stdev: 20.448 ms (10.5%) Runs: 168 169 169 171 176 177 177 177 183 184 184 187 188 188 191 193 194 195 195 197 198 202 209 209 212 214 217 221 222 239 254
App start regularAppStart	Baseline Mean: 0.014 ms Stdev: 0.001 ms (4.5%) Runs: 0.012980000115931034 0.013019999489188194 0.013265000656247139 0.013386999256908894 0.013387000188231468 0.013387000188231468 0.013387000188231468 0.0134680001065135 0.013469000346958637 0.013590999878942966 0.013793999329209328 0.013833999633789062 0.01387499924749136 0.013875000178813934 0.013875000178813934 0.013875000178813934 0.013915999792516232 0.013956000097095966 0.013956999406218529 0.0139979999512434 0.014078999869525433 0.01416000071913004 0.014201000332832336 0.014322999864816666 0.014526000246405602 0.014689000323414803 0.0147299999371171 0.0147299999371171 0.014972999691963196 0.01534000039100647 0.015665000304579735 Current Mean: 0.015 ms Stdev: 0.001 ms (6.9%) Runs: 0.013712999410927296 0.01375299971550703 0.013794000260531902 0.013997000642120838 0.014038000255823135 0.014038000255823135 0.014038000255823135 0.01416000071913004 0.014199999161064625 0.014241000637412071 0.014485999941825867 0.014566999860107899 0.01464799977838993 0.0148930000141263 0.0148930000141263 0.0148930000141263 0.014973999932408333 0.015137000009417534 0.015300000086426735 0.015339999459683895 0.015340999700129032 0.015420999377965927 0.01550300046801567 0.015868999995291233 0.016030999831855297 0.01607200037688017 0.016195000149309635 0.0163569999858737 0.016560999676585197 0.0170499999076128 0.017170999199151993 0.017416000366210938
App start nativeLaunch	Baseline Mean: 20.406 ms Stdev: 2.044 ms (10.0%) Runs: 18 18 18 18 18 18 18 19 19 19 19 19 19 20 20 20 20 21 21 21 21 21 21 22 22 22 22 22 23 23 25 26 Current Mean: 20.097 ms Stdev: 2.022 ms (10.1%) Runs: 17 18 18 18 18 18 19 19 19 19 19 19 19 19 19 19 19 20 20 20 21 21 21 21 22 22 23 23 24 24 25
Open Search Page TTI	Baseline Mean: 604.926 ms Stdev: 25.860 ms (4.3%) Runs: 545.3155109994113 569.0140789998695 579.7418619999662 581.2392980000004 585.900391000323 586.2559409998357 586.4849049998447 587.3820400005206 588.3706460008398 588.5419109994546 588.5566409993917 589.0607510004193 589.8441169997677 590.9528810000047 595.624715000391 595.7393800001591 602.0906990002841 602.262085000053 603.6059980001301 605.2273359997198 605.5517589999363 613.8769939998165 616.8317880006507 618.5505790002644 622.6282959999517 623.2633870001882 625.1344410004094 625.9578860001639 631.3835859997198 635.609416000545 654.7382810004056 661.8194989999756 666.0125730000436 Current Mean: 604.296 ms Stdev: 15.952 ms (2.6%) Runs: 573.334268999286 574.9369719997048 583.750326000154 586.8765059998259 591.1694750003517 592.2617999995127 593.2267260001972 593.3871659999713 594.4565019998699 594.9581300001591 595.0317789996043 595.1517340000719 595.6909590000287 596.344076000154 598.4178059995174 600.3108320003375 601.4043379994109 601.5808920003474 604.0057779997587 605.6188559997827 605.7046720003709 606.6571460003033 609.9432379994541 618.0411789994687 618.2456459999084 619.7016600007191 620.0800780002028 620.6228849999607 623.0149750001729 623.8829340003431 628.6651610005647 635.8930669995025 639.386800000444

[OSBotify]

🚀 Deployed to staging by https://github.com/mountiny in version: 1.2.61-0 🚀

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

[OSBotify]

🚀 Deployed to production by https://github.com/thienlnam in version: 1.2.61-0 🚀

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