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.

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] Update Expensify-Playbook-for-US-based-VC-Backed-Startups.md #15308

Merged
merged 3 commits into from
Feb 21, 2023

Conversation

zsgreenwald
Copy link
Contributor

@zsgreenwald zsgreenwald commented Feb 20, 2023

I ran the initial doc through Grammarly and made some revisions + fixed the issue with double quoting the quotes in this article.

Details

Fixed Issues

$ GH_LINK
PROPOSAL: GH_LINK_ISSUE(COMMENT)

Tests

  • Verify that no errors appear in the JS console

Offline tests

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 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.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android

I ran the initial doc through Grammarly and made some revisions + fixed the issue with double quoting the quotes in this article.
@zsgreenwald zsgreenwald requested a review from a team as a code owner February 20, 2023 20:03
@melvin-bot melvin-bot bot requested review from joelbettner and removed request for a team February 20, 2023 20:03
@MelvinBot
Copy link

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

Changed the naming of Continuous Reconciliation to Auto-Reconciliation
@joelbettner
Copy link
Contributor

joelbettner commented Feb 21, 2023

This only touches a readme file, and not any code. I'm going to check everything off the list.

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.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

image

Mobile Web - Chrome

image

Mobile Web - Safari

image

Desktop

image

iOS

image

Android

image

@marcochavezf marcochavezf changed the title Update Expensify-Playbook-for-US-based-VC-Backed-Startups.md [No QA] Update Expensify-Playbook-for-US-based-VC-Backed-Startups.md Feb 21, 2023
@marcochavezf
Copy link
Contributor

Adding No QA since it's only a grammar/text change

@joelbettner joelbettner merged commit ba7c381 into main Feb 21, 2023
@joelbettner joelbettner deleted the zsgreenwald-patch-1 branch February 21, 2023 22:39
@OSBotify
Copy link
Contributor

✋ 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
Copy link
Contributor

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries
Name Duration
App start TTI 731.320 ms → 743.976 ms (+12.656 ms, +1.7%)
App start runJsBundle 201.161 ms → 206.677 ms (+5.516 ms, +2.7%)
App start regularAppStart 0.015 ms → 0.015 ms (+0.000 ms, +1.8%)
App start nativeLaunch 19.875 ms → 19.406 ms (-0.469 ms, -2.4%)
Open Search Page TTI 609.560 ms → 607.227 ms (-2.333 ms, ±0.0%)
Show details
Name Duration
App start TTI Baseline
Mean: 731.320 ms
Stdev: 26.883 ms (3.7%)
Runs: 668.7850510003045 687.0548729998991 693.0316099999472 694.6477720001712 698.7722319997847 702.5624200003222 709.388151999563 711.8862199997529 714.1803310001269 717.4076779996976 724.9990839995444 726.8339459998533 731.3260169997811 731.767675999552 732.1120159998536 732.1654160004109 732.6888960003853 733.3027560003102 737.350437999703 741.5775279998779 741.6366429999471 746.1035759998485 748.1746800001711 750.94550300017 752.3586269998923 752.5245949998498 760.2290350003168 764.787092000246 773.3969529997557 775.4039150001481 783.5232739998028

Current
Mean: 743.976 ms
Stdev: 25.701 ms (3.5%)
Runs: 695.1271360004321 702.6207630001009 708.867428000085 709.0747370002791 712.1168440002948 712.9431029995903 716.5345620000735 717.5234179999679 719.0213550003245 723.4509479999542 730.776344999671 739.8836359996349 743.1011469997466 745.9988839998841 746.653376000002 748.8329699998721 750.339925000444 751.7517299996689 752.3472880003974 753.4006599998102 756.145480000414 759.0159769998863 759.2048350004479 759.246899000369 759.6226479997858 760.0462910002097 762.0711730001494 771.3164750002325 771.424199000001 777.795556999743 793.4813130004331 797.4866239996627
App start runJsBundle Baseline
Mean: 201.161 ms
Stdev: 18.858 ms (9.4%)
Runs: 171 176 177 177 177 179 179 184 184 196 197 198 198 199 199 200 201 203 206 207 208 208 209 212 216 218 219 220 230 238 250

Current
Mean: 206.677 ms
Stdev: 15.260 ms (7.4%)
Runs: 179 183 184 186 191 192 193 195 196 197 198 202 204 204 206 207 207 207 208 211 213 214 214 216 220 225 225 227 231 233 239
App start regularAppStart Baseline
Mean: 0.015 ms
Stdev: 0.001 ms (6.0%)
Runs: 0.013345999643206596 0.013508999720215797 0.01371300034224987 0.013875000178813934 0.013875000178813934 0.013957000337541103 0.014078999869525433 0.014404000714421272 0.014444999396800995 0.014486000873148441 0.014526999555528164 0.014526999555528164 0.014566999860107899 0.014648000709712505 0.014649000018835068 0.014933999627828598 0.01505500078201294 0.015056000091135502 0.015096000395715237 0.015341000631451607 0.015422000549733639 0.015542999841272831 0.015542999841272831 0.015625 0.015665000304579735 0.0157880000770092 0.015869999304413795 0.01664199959486723 0.017211999744176865

Current
Mean: 0.015 ms
Stdev: 0.001 ms (6.8%)
Runs: 0.01318300049751997 0.013670999556779861 0.01375299971550703 0.013834000565111637 0.013957000337541103 0.014119000174105167 0.014159999787807465 0.014281999319791794 0.014485999941825867 0.014526999555528164 0.014566999860107899 0.014566999860107899 0.0147299999371171 0.014810999855399132 0.014811000786721706 0.014933000318706036 0.015217999927699566 0.015218999236822128 0.015258999541401863 0.015584999695420265 0.015625 0.015666000545024872 0.015747000463306904 0.015828999690711498 0.015910000540316105 0.016193999908864498 0.016276000067591667 0.016276000067591667 0.016315999440848827 0.01680499967187643 0.017090000212192535 0.01729400083422661
App start nativeLaunch Baseline
Mean: 19.875 ms
Stdev: 1.495 ms (7.5%)
Runs: 18 18 18 18 18 18 18 19 19 19 19 19 19 19 20 20 20 20 20 20 20 20 20 21 21 21 22 22 22 22 23 23

Current
Mean: 19.406 ms
Stdev: 1.518 ms (7.8%)
Runs: 17 18 18 18 18 18 18 18 18 18 18 19 19 19 19 19 19 19 19 20 20 20 20 20 20 20 21 21 22 22 23 23
Open Search Page TTI Baseline
Mean: 609.560 ms
Stdev: 15.577 ms (2.6%)
Runs: 584.5679930001497 587.4975180001929 590.1057540001348 590.2618819996715 591.2303879996762 594.625284999609 594.9828289998695 596.425578000024 597.9912520004436 600.6955979997292 601.2232260005549 601.6087249992415 601.7150070006028 602.8707280000672 605.6767990002409 606.3665359998122 606.5603840006515 607.0361329996958 615.100260999985 615.6221920000389 617.1717939991504 617.809407999739 623.028605999425 623.5788580002263 624.9912520004436 626.2476000003517 629.3663739999756 631.5526119992137 633.2668049996719 636.191487999633 641.0006919996813

Current
Mean: 607.227 ms
Stdev: 19.128 ms (3.2%)
Runs: 569.9171139998361 576.8184409998357 578.2818200001493 579.9697679998353 589.3809820003808 591.6424569999799 593.8828119998798 594.390625 594.4267579996958 594.8313400000334 596.1827400000766 599.9564619995654 599.9672849997878 601.1351320007816 603.9050300000235 604.0637619998306 606.6512040002272 606.6653649993241 606.7625739993528 608.1563320001587 608.8316239994019 609.2644449993968 613.8179529998451 616.4614260001108 616.8509110007435 620.8923750007525 630.672159999609 632.2545170001686 633.3685710001737 633.9215089995414 637.0729169994593 637.8101410008967 650.2884929999709

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by https://github.com/joelbettner in version: 1.2.76-0 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by https://github.com/yuwenmemon in version: 1.2.76-7 🚀

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

1 similar comment
@OSBotify
Copy link
Contributor

🚀 Deployed to production by https://github.com/yuwenmemon in version: 1.2.76-7 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by https://github.com/yuwenmemon in version: 1.2.76-7 🚀

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants