Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Clean up linting configuration #268

Merged
merged 1 commit into from
Jul 20, 2020
Merged

Clean up linting configuration #268

merged 1 commit into from
Jul 20, 2020

Conversation

edmorley
Copy link
Member

  • Switch back from babel-eslint to the default ESLint parser, since it supports all of the syntax we are using out of the box.
  • Use the new ignorePatterns option instead of .eslintignore.
  • Adjust the ignore pattern from !.eslintrc.js to !.*.js so that all dotfiles are un-ignored, which also allows us to remove the multiple globbing combinations in the ESLint CLI command used for yarn lint, and instead pass just ..
  • Use the new reportUnusedDisableDirectives option instead of the --report-unused-disable-directives CLI flag.
  • Enable --max-warnings 0 so that CI fails for warnings too.
  • Move the Prettier config out of .eslintrc.js to .prettierrc.js, since not all editor integrations check the ESLint config location.
  • Switch from running Prettier as an ESLint plugin to using the native CLI, since it's faster, the UX is better when using editor integrations (eg prevents lots of red underlines which will be fixed on save anyway), and it means non-JS file types can be formatted. Additional package.json scripts have been added to make it easier for those who don't use format-on-save to apply fixes.
  • The previously unchecked files (*.json and files under types/) have had their formatting fixed using yarn fix.

* Switch back from `babel-eslint` to the default ESLint parser, since
  it supports all of the syntax we are using out of the box.
* Use the new `ignorePatterns` option instead of `.eslintignore`.
* Adjust the ignore pattern from `!.eslintrc.js` to `!.*.js` so that
  all dotfiles are ignored by default, which also allows us to remove
  the multiple globbing combinations in the ESLint CLI command used for
  `yarn lint`, and instead pass just `.`.
* Use the new `reportUnusedDisableDirectives` option instead of the
  `--report-unused-disable-directives` CLI flag.
* Enable `--max-warnings 0` so that CI fails for warnings too.
* Move the Prettier config out of `.eslintrc.js` to `.prettierrc.js`,
  since not all editor integrations check the ESLint config location.
* Switch from running Prettier as an ESLint plugin to using the native
  CLI, since it's faster, the UX is better when using editor
  integrations (eg prevents lots of red underlines which will be fixed
  on save anyway), and it means non-JS file types can be formatted.
  Additional `package.json` scripts have been added to make it easier
  for those who don't use format-on-save to apply fixes.
* The previously unchecked files (`*.json` and files under `types/`)
  have had their formatting fixed using `yarn fix`.
@edmorley edmorley self-assigned this Jul 19, 2020
@edmorley edmorley requested a review from a team July 19, 2020 16:04
Copy link
Member

@eliperelman eliperelman left a comment

Choose a reason for hiding this comment

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

LGTM! The one thing I am concerned about is moving away from babel-eslint, as it will make the switch back to it a little less fun, but I think it's probably the right choice. Nevermind, was looking at the wrong repo. 😄

@edmorley
Copy link
Member Author

Re the switch away from babel-eslint, my thoughts were that if we end up using non-standard syntax, we'll have to use add a build step for transpiling that (particularly given webpack-chain supports more Node versions than say Neutrino) - and so (a) it might mean we're less inclined to do that, (b) even if we do, we'll require a bunch of changes to add the build step, and we can switch back to babel-eslint at that point :-)

@edmorley edmorley merged commit 74b1791 into master Jul 20, 2020
@edmorley edmorley deleted the edmorley-cleanup-linting branch July 20, 2020 15:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

2 participants