[tests] Unify linting and autoformatting #2914
Conversation
lucleray
requested review from
leo,
rdev,
styfle and
TooTallNate
as code owners
Codecov Report
@@ Coverage Diff @@
## canary #2914 +/- ##
==========================================
- Coverage 11.06% 11.04% -0.02%
==========================================
Files 267 267
Lines 10133 10133
Branches 1264 1266 +2
==========================================
- Hits 1121 1119 -2
- Misses 8922 8924 +2
Partials 90 90
Continue to review full report at Codecov.
|
lucleray
requested review from
AndyBitz,
coetry,
thasophearak and
Timer
as code owners
|
@rdev Improved eslint configuration and fixed some errors: |
styfle
changed the title
|
Could we publish an eslint plugin that uses the prettier plugin and add instructions for a standard ZEIT linting setup? |
| }; | ||
|
|
||
| // Static builders are special cased in `@now/static-build` | ||
| function getBuilders(): Map<string, Builder> { | ||
| return new Map<string, Builder>([ | ||
| ['next', { src, use: '@now/next', config }], | ||
| ['next', { src, use: '@now/next', config }] |
There was a problem hiding this comment.
Why don't we continue using trailing commas?
There was a problem hiding this comment.
There was no trailing comma in now-cli so I went for no trailing comma.
There was a problem hiding this comment.
@lucleray Tbh I prefer trailing commas because of the cleaner diffs 🤔
But it's not a strong stance
There was a problem hiding this comment.
I personally think trailing commas give cleaner diffs, what do you think @styfle
There was a problem hiding this comment.
I agree. I like trailing commas because they're less noise when reviewing PRs.
In this case, however, let's try to keep now-cli consistent because that was the original repo and has all the history here.
There was a problem hiding this comment.
Also because of prettier right?
There was a problem hiding this comment.
prettier allows you to configure trailing commas
|
@lucleray What do you think about unifying the |
* add prettier and eslint on root * remove eslint from now-cli * adjust root package.json * adjust eslintignore * adjust now-cli rules * remove @zeit/git-hooks in packages * adjust now-client eslint config * add lint-staged and hook on pre-commit * add pre-commit script * replace @zeit/git-hooks with husky * remove unnecessary script * fix eslint errors * trigger tests * fix fixable errors * fix fixable errors (bis) * revert two changes
This PR adds:
prettierto autoformateslinton the root folder to unify it across the repositorylint-stagedto improve our code incrementally (only files that are touched in the commit are linted/prettified)huskyto add a hook on pre-commitThere are still many lint errors across the code base (
yarn run lintto list them). Because of that, onlynow-clientandnow-cliare currently part of the CI step, but please add others packages when they pass lint!To add a package to the CI step, simply add a script to the package:
{ "scripts": { "test-lint": "eslint . --ext .ts,.js --ignore-path ../../.eslintignore" } }Eventually, we'll have all packages linted and we can simply run
eslintfrom the root package.Remarks:
.eslintrc.jsonif a rule is not appropriate or causing too much problems. Please tweak the rule only for the specific package and not for the whole monorepo (unless necessary). The goal is to unify linting so please don't add too many rules 😅eslint-config-airbnbbecause it is bloated with confusing rules and i believe we are smart programmers 🤓)Todo:
now-clinow-clinow-clientFixes #2912