Skip to content

Add item in the PR author list to make sure copy changes are correct - #14288

Merged
ctkochan22 merged 2 commits into
mainfrom
rocio-RequestedCopy
Jan 13, 2023
Merged

ctkochan22 merged 2 commits into
mainfrom
rocio-RequestedCopy

Conversation

@pecanoro

@pecanoro pecanoro commented Jan 13, 2023

Copy link
Copy Markdown
Contributor

Details

As mentioned here, we are adding an item so copy changes are correct when added in Spanish.

Fixed Issues

N/A

Tests

image

  • Verify that no errors appear in the JS console

Offline tests

N/A

QA Steps

N/A

  • 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
    • 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

Web

N/A

Mobile Web - Chrome

N/A

Mobile Web - Safari

N/A

Desktop

N/A

iOS

N/A

Android

N/A

@pecanoro
pecanoro requested a review from a team as a code owner January 13, 2023 16:35
@melvin-bot
melvin-bot Bot requested review from ctkochan22 and removed request for a team January 13, 2023 16:35
@melvin-bot

melvin-bot Bot commented Jan 13, 2023

Copy link
Copy Markdown

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

@ctkochan22
ctkochan22 merged commit ba28757 into main Jan 13, 2023
@ctkochan22
ctkochan22 deleted the rocio-RequestedCopy branch January 13, 2023 22:19
@OSBotify

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

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries
Name Duration
Open Search Page TTI 610.032 ms → 619.102 ms (+9.070 ms, +1.5%)
App start TTI 673.901 ms → 682.467 ms (+8.566 ms, +1.3%)
App start runJsBundle 184.226 ms → 187.031 ms (+2.805 ms, +1.5%)
App start nativeLaunch 19.267 ms → 19.900 ms (+0.633 ms, +3.3%)
App start regularAppStart 0.020 ms → 0.021 ms (+0.000 ms, +1.0%)
Show details
Name Duration
Open Search Page TTI Baseline
Mean: 610.032 ms
Stdev: 21.040 ms (3.4%)
Runs: 570.1658930000849 575.4172769999132 577.6222339998931 579.3424889999442 583.0445559998043 585.7483319998719 587.8293459997512 588.3926189998165 590.6975910002366 595.218423999846 597.4372970000841 605.3677570000291 609.2432460002601 610.0135499997996 612.0447600004263 612.2117519997992 612.2371419998817 616.8503419999033 617.1334640001878 618.7391770002432 620.8933109999634 623.2799070002511 623.975097999908 624.2102060001343 627.5077720000409 629.2868249998428 633.0904139997438 634.4160569999367 634.8263350003399 638.3276780000888 638.9965420002118 647.463623999618

Current
Mean: 619.102 ms
Stdev: 23.309 ms (3.8%)
Runs: 576.8028569999151 578.2036949996836 586.3501800000668 588.0580649999902 589.7491870000958 593.5734870000742 595.2980549996719 595.6031499998644 595.6078699999489 604.2898769997992 609.9991049999371 610.6380620002747 612.665608999785 614.7485759998672 615.6694340002723 616.3630369999446 620.1332200001925 623.2918699998409 624.1236570002511 626.1894540004432 629.463339000009 630.4276939998381 630.7541919997893 634.5952960001305 636.6992190000601 638.9559330004267 641.722453000024 642.90393100027 646.2225339999422 646.3037919998169 646.5303150000982 662.3618580000475 666.0752769997343
App start TTI Baseline
Mean: 673.901 ms
Stdev: 24.822 ms (3.7%)
Runs: 623.9534749998711 629.386369000189 636.2692920002155 640.5119570000097 648.6315839998424 648.8489069999196 661.2337650000118 661.6167210000567 662.3096210001968 664.4038510001265 664.703619999811 667.4535840000026 671.122744999826 671.2626419998705 672.1201309999451 672.32773800008 673.5839700000361 678.8812989997678 678.901078000199 680.5207899999805 681.0951890000142 686.8638089997694 691.0744309998117 695.5135750002228 695.5850869999267 698.5666780001484 702.8798250001855 706.1972030000761 717.1255959998816 734.0869049998

Current
Mean: 682.467 ms
Stdev: 24.707 ms (3.6%)
Runs: 633.3686500000767 643.4540679999627 644.2526130001061 646.1479480001144 652.2664129999466 654.6382380002178 655.7965529998764 663.2613809998147 665.259387999773 670.8323019999079 672.3733319998719 672.713533999864 673.6578489998356 675.9593420000747 680.2092059999704 682.18335200008 686.1121519999579 686.7195839998312 690.3378800000064 692.4313460001722 694.4641689998098 697.56481300015 697.6410269998014 697.676738999784 699.4096949999221 710.5125660002232 711.6036840002052 712.4783370001242 713.5767649998888 714.6753699998371 718.6644230000675 728.6948520001024
App start runJsBundle Baseline
Mean: 184.226 ms
Stdev: 16.439 ms (8.9%)
Runs: 157 159 159 167 167 168 168 170 173 174 176 178 178 179 181 184 185 186 187 188 192 193 193 197 199 202 202 205 206 214 224

Current
Mean: 187.031 ms
Stdev: 17.551 ms (9.4%)
Runs: 159 168 169 169 170 171 171 173 174 176 177 177 178 179 179 179 183 183 184 186 194 195 196 197 200 202 207 210 211 213 220 235
App start nativeLaunch Baseline
Mean: 19.267 ms
Stdev: 1.436 ms (7.5%)
Runs: 17 17 17 18 18 18 18 18 18 19 19 19 19 19 19 19 19 19 19 20 20 20 20 20 21 21 21 21 22 23

Current
Mean: 19.900 ms
Stdev: 1.469 ms (7.4%)
Runs: 18 18 18 18 18 19 19 19 19 19 19 19 19 19 20 20 20 20 20 20 21 21 21 21 21 21 21 22 23 24
App start regularAppStart Baseline
Mean: 0.020 ms
Stdev: 0.002 ms (11.1%)
Runs: 0.01639900030568242 0.0181470001116395 0.01814799988642335 0.018188000191003084 0.018514000345021486 0.0186769999563694 0.0188400000333786 0.01896099979057908 0.019246999640017748 0.019286999944597483 0.019368999637663364 0.019450000021606684 0.019652999937534332 0.019775000400841236 0.019776000175625086 0.01985699962824583 0.020264000166207552 0.020264000166207552 0.020752000156790018 0.020793000236153603 0.02091500023379922 0.021037000231444836 0.021321000065654516 0.02144400030374527 0.022582999896258116 0.02282699989154935 0.023233999963849783 0.024129000026732683 0.024861000012606382 0.02750599989667535

Current
Mean: 0.021 ms
Stdev: 0.001 ms (6.4%)
Runs: 0.018106999807059765 0.018961000256240368 0.019286999944597483 0.019328000023961067 0.0194089999422431 0.0194089999422431 0.019531999714672565 0.019694000016897917 0.0197350000962615 0.019857000093907118 0.020223000086843967 0.020345000084489584 0.020426000002771616 0.02046799985691905 0.020629999693483114 0.020793000236153603 0.020873999688774347 0.02091500023379922 0.02103699976578355 0.021077999845147133 0.02107800031080842 0.0211990000680089 0.021240000147372484 0.02164700021967292 0.021688000299036503 0.02254199981689453 0.02262300020083785 0.02343800012022257 0.024128999561071396

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by @ctkochan22 in version: 1.2.55-0 🚀

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

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by @AndrewGable in version: 1.2.55-0 🚀

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.

3 participants