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

🏛 fix: quiet logs when linting in CI #4070

Conversation

estrattonbailey
Copy link
Contributor

Fixes RNBW-4331

What changed (plus any additional context for devs)

With the new ESLint setup, we have >4k warnings. These only useful when commiting changes locally via the lint-staged pre-commit git hook. In CI, we should only surface errors. This is what the --quiet option does.

Since yarn's handling of passed arguments to npm scripts might not be clear to everyone, I opted to spell out the whole command in the GitHub Action instead of yarn lint --quiet.

Final checklist

  • Assigned individual reviewers?
  • Added labels? (team1/team2, critical path, release, dev QA)
  • Did you test both iOS and Android?
  • If your changes are visual, did you check both the light and dark themes?
  • Added e2e tests? If not, please specify why
  • If you added new critical path files, did you update the CODEOWNERS file?
  • If no dev QA label, did you add the PR to the QA Queue?

@linear
Copy link

linear bot commented Aug 24, 2022

RNBW-4331 Use recommended ESLint settings

At the moment we've manually configured ESLint rules (almost 400 lines worth) and ignored the recommended settings. This is swallowing a handful of errors that really should not be ignored, like:

  • @typescript-eslint/no-non-null-asserted-optional-chain which is dangerous and can lead to bugs like RNBW-4283
  • @typescript-eslint/ban-types where we mix up Boolean and boolean etc
  • no-async-promise-executor where we have async functions within Promise which is already async, and can lead to race conditions
  • @typescript-eslint/no-unnecessary-type-constraint which might alter the call signature of a function in unexpected ways
  • error on no-useless-catch because usually we're doing this to capture an error and we should capture an error
  • @typescript-eslint/ban-ts-comment which should be set to warn so that we can work towards full type coverage
  • no-dupe-if-else is an important one to throw, and we were swallowing it
  • sorting our keys is a waste of time — we had over 70 overrides, see commit — if we really want this we should have prettier do it

Based on my tinkering, we can improve this situation fairly quickly.

RNBW-4371 Use `--quiet` option when linting in CI

Base automatically changed from eric/rnbw-4331-use-recommended-eslint-settings to develop August 25, 2022 18:23
@estrattonbailey estrattonbailey force-pushed the eric/rnbw-4371-use---quiet-option-when-linting-in-ci branch from 5c59d7b to 4d39d05 Compare August 25, 2022 18:51
@estrattonbailey estrattonbailey changed the title fix: quiet logs when linting in CI 🏛 fix: quiet logs when linting in CI Aug 25, 2022
Copy link
Contributor

@skylarbarrera skylarbarrera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏛️

@estrattonbailey estrattonbailey merged commit 1c07470 into develop Sep 2, 2022
@estrattonbailey estrattonbailey deleted the eric/rnbw-4371-use---quiet-option-when-linting-in-ci branch September 2, 2022 20:06
estrattonbailey added a commit that referenced this pull request Sep 6, 2022
…w-4378-clean-up-global-and-env-vars-in-app-move

* 'develop' of github.com:rainbow-me/rainbow: (40 commits)
  fix: quiet logs when linting in CI (#4070)
  analytics: add tracking for screen dimensions + fix nav events on android (#4088)
  fix native input cursor color (#4112)
  Profiles: use `name` text record instead of `me.rainbow.displayName` (#4046)
  chore: upgrade eslint-config-rainbow (#4118)
  backup PIN  (#4012)
  Remove long press on address in switching wallets (#3990)
  Version bump iOS v.1.7.4 & Android 144 (#4117)
  Fix slow image loading (#4052)
  Add new text size scale, deprecate old sizes and Heading (#4113)
  Disable campaings in e2e and fix testID of PromoSheet (#4107)
  🏛 chore: prettier everything (#4100)
  fix settings StatusIcon shadows on android (#4104)
  fix clear async storage crash (#4105)
  fix RNBW-4416 (#4103)
  🏛 Update to React Query 4 (#4060)
  swaps: init cross chain feature flag (#4091)
  enable arbiitrum swap details test (#4092)
  remove unsafe access to networkInfo (#4096)
  network check (#4094)
  ...
estrattonbailey added a commit that referenced this pull request Sep 6, 2022
…w-4388-configure-jest-for-unit-tests

* 'develop' of github.com:rainbow-me/rainbow:
  Profiles: Support contenthash (#4067)
  🏛 GraphQL Clients/Codegen Foundations (#4086)
  fix: quiet logs when linting in CI (#4070)
  analytics: add tracking for screen dimensions + fix nav events on android (#4088)
  fix native input cursor color (#4112)
  Profiles: use `name` text record instead of `me.rainbow.displayName` (#4046)
  chore: upgrade eslint-config-rainbow (#4118)
  backup PIN  (#4012)
  Remove long press on address in switching wallets (#3990)
  Version bump iOS v.1.7.4 & Android 144 (#4117)
  Fix slow image loading (#4052)
  Add new text size scale, deprecate old sizes and Heading (#4113)
estebanmino pushed a commit that referenced this pull request Sep 9, 2022
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