Add prettier + eslint #89
Conversation
fathyb
force-pushed
the
chore/add-prettier
branch
from
8042641 to
12f490a
Compare
| test: | ||
| docker: | ||
| - image: circleci/node:4-browsers | ||
| - image: circleci/node:8-browsers |
There was a problem hiding this comment.
Had to upgrade to 8 for lint-staged, see CI Job#14
There was a problem hiding this comment.
Would this have any effect on the final functionality?
There was a problem hiding this comment.
The final artifact is built by the analytics.js repo so it should only affect tests
fathyb
force-pushed
the
chore/add-prettier
branch
from
12f490a to
50b6f86
Compare
| @@ -1,3 +1,3 @@ | |||
| { | |||
| "extends": "@segment/eslint-config/browser/legacy" | |||
There was a problem hiding this comment.
Do you think we still need to keep the old config around?
There was a problem hiding this comment.
I think we should, browser/legacy ensures we are only using ES3 compatible code which we can't really guarantee otherwise given we don't use any transpilers
There was a problem hiding this comment.
Gotcha, that makes sense - we definitely need that 🙏
|
|
||
| # Lint JavaScript source files. | ||
| lint: install | ||
| @$(ESLINT) $(ALL_FILES) |
There was a problem hiding this comment.
If we're not using ESLint anymore in the makefile, we could remove it from line 5 as well.
There was a problem hiding this comment.
Today's free tip: with newer versions of yarn you can directly invoke bin programs.
Example: yarn codecov instead of ./node_modules/.bin/codecov
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "singleQuote": true | |||
There was a problem hiding this comment.
Is this needed? What does it do? I see our internal app codebase using single quotes but not have this prettier rule.
There was a problem hiding this comment.
Ah nvm, they pass it as an option in the binary instead: /prettier --write --single-quote --no-semi 'src/**/*.js'. I like putting it the config better 👍
There was a problem hiding this comment.
If we remove it prettier will use double quotes when formatting. It could also be in .eslintrc and package.json IIRC. I've put it in this file so editor plugins can use this setting.
f2prateek
left a comment
f2prateek
left a comment
There was a problem hiding this comment.
couple of questions but looks great overall.
2 participants
This PR adds
prettier-eslint, which is justprettierwith its output piped toeslint --fix.jsfiles are formatted usingeslintrules, whilemdandjsonfiles only usingprettier.huskyis added to setup a Gitprecommithook, andlint-stagedto only check staged files. This will make sure every commit is properly formatted and passes the lint test. This can be bypassed by using the Gitcommit--no-verifyoption.A
formatscript is added to runprettieron all files.https://segment.atlassian.net/browse/LIB-413
cc @f2prateek