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

ci: add prettier back 🎉 #764

Merged
merged 6 commits into from
Jul 31, 2020
Merged

Conversation

nickofthyme
Copy link
Collaborator

@nickofthyme nickofthyme commented Jul 23, 2020

Summary

🚧 Waiting for #751 before merging 🚧

closes #702

  • conflicting rules typescript-eslint/member-ordering and react/sort-comp
  • max-len rule not auto fixable.
  • Switch back to prettier for indenting
  • eslint formatOnSave vscode configuration (eslint/indent vs eslint/prettier)
  • split out high load rules to only run on yarn lint

my vscode settings.json config for eslint is below. Some of which is not needed for ec like vue but doesn't hurt anything.

{
  "eslint.codeAction.showDocumentation": {
    "enable": true
  },
  "eslint.codeAction.disableRuleComment": {
    "enable": true,
    "location": "separateLine"
  },
  "eslint.validate": [
    "javascript",
    "javascriptreact",
    "vue",
    "html",
    "typescript",
    "typescriptreact",
  ],
}

full settings options available here

Removed most cpu intensive rules from dev config. @typescript-eslint/* rules seem to lump together, no matter how many I remove another @typescript-eslint/* rule will show up at the top of the list. Current excluded rules are:

  • import/no-restricted-paths
  • import/namespace
Rule                                    | Time (ms) | Relative
:---------------------------------------|----------:|--------:
prettier/prettier                       | 15441.700 |    36.1%
@typescript-eslint/no-floating-promises |  9862.220 |    23.1%
@typescript-eslint/no-misused-promises  |  1964.450 |     4.6%
react/no-unknown-property               |  1469.325 |     3.4%
import/no-unresolved                    |  1135.063 |     2.7%
react/prefer-stateless-function         |  1037.916 |     2.4%
react/void-dom-elements-no-children     |   662.293 |     1.5%
@typescript-eslint/unbound-method       |   623.038 |     1.5%
react/no-deprecated                     |   482.789 |     1.1%
react/no-direct-mutation-state          |   475.650 |     1.1%

@codecov-commenter
Copy link

Codecov Report

Merging #764 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #764   +/-   ##
=======================================
  Coverage   74.45%   74.45%           
=======================================
  Files         269      269           
  Lines        9303     9303           
  Branches     1936     1936           
=======================================
  Hits         6927     6927           
  Misses       2371     2371           
  Partials        5        5           
Flag Coverage Δ
#unittests 74.45% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f2e35d...e8a47c9. Read the comment docs.

@nickofthyme nickofthyme marked this pull request as ready for review July 30, 2020 18:32
Copy link
Member

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

LGTM!

.eslintrc.js Show resolved Hide resolved
@nickofthyme nickofthyme merged commit 35248d1 into elastic:master Jul 31, 2020
@nickofthyme nickofthyme deleted the add-prettier-back branch July 31, 2020 12:55
@markov00
Copy link
Member

🎉 This PR is included in version 21.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@markov00 markov00 added the released Issue released publicly label Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ci released Issue released publicly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linting configuration changes
3 participants