[NO QA] chore(deps): bump eslint-config-expensify to 2.0.99#75887
Conversation
|
@shubham1206agra 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] |
|
|
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
| // eslint-disable-next-line rulesdir/no-default-id-values | ||
| const policyID = policy?.id ?? '-1'; |
There was a problem hiding this comment.
This unrelated Changed files ESLint check error happens when I add unicorn/no-array-for-each inline disable to this file.
I disabled this error because I’m not sure how to address them.
Error: 21:32 error Do not default string IDs to any value. See: https://github.com/Expensify/App/blob/main/contributingGuides/STYLE.md#default-value-for-inexistent-IDs rulesdir/no-default-id-values
There was a problem hiding this comment.
Did you read the documentation at https://github.com/Expensify/App/blob/main/contributingGuides/STYLE.md#default-value-for-inexistent-IDs ? I think it's pretty clear. Remove the ?? '-1' and update subsequent uses to correctly account for policyID being potentially undefined.
There was a problem hiding this comment.
thanks for helping
| // eslint-disable-next-line no-restricted-syntax | ||
| import * as ErrorUtils from '@libs/ErrorUtils'; |
There was a problem hiding this comment.
This unrelated Changed files ESLint check error happens when I add unicorn/no-array-for-each inline disable to this file.
I disabled this error because I’m not sure how to address them.
Error: 8:8 error Namespace imports from @libs are not allowed. Use named imports instead. Example: import { method } from "@libs/module" no-restricted-syntax
There was a problem hiding this comment.
I personally really dislike this lint rule, but we should fix it rather than disabling it. You need to look at usages of ErrorUtils in this file, destructure all the imports that are used in this file, and then remove the ErrorUtils. prefix from all usages.
There was a problem hiding this comment.
thanks for helping.
done destructuring methods of ErrorUtils
|
@roryabraham ready for review |
| // eslint-disable-next-line no-restricted-syntax | ||
| import * as ErrorUtils from '@libs/ErrorUtils'; |
There was a problem hiding this comment.
I personally really dislike this lint rule, but we should fix it rather than disabling it. You need to look at usages of ErrorUtils in this file, destructure all the imports that are used in this file, and then remove the ErrorUtils. prefix from all usages.
| // eslint-disable-next-line rulesdir/no-default-id-values | ||
| const policyID = policy?.id ?? '-1'; |
There was a problem hiding this comment.
Did you read the documentation at https://github.com/Expensify/App/blob/main/contributingGuides/STYLE.md#default-value-for-inexistent-IDs ? I think it's pretty clear. Remove the ?? '-1' and update subsequent uses to correctly account for policyID being potentially undefined.
5cd36f0 to
50ce0f0
Compare
50ce0f0 to
7919425
Compare
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
|
Congrats, that's your 5th PR merged! 🎉 Do you know about the ContributorPlus role? It's an opportunity to earn more in the Expensify Open Source community. Keep up the great work - thanks! |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.2.64-0 🚀
|
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.2.64-5 🚀
|
Explanation of Change
eslint-config-expensifyto 2.0.99unicorn/no-array-for-eachviolations (will be fixed in my follow-up PRs)Fixed Issues
$ #67422
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop