Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upupgrades eslint v5 (major), babel-eslint, eslint-plugin-flowtype, eslint-plugin-prettier #8259
Conversation
This comment has been minimized.
This comment has been minimized.
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/8489/ |
| @@ -47,10 +47,10 @@ flow: | |||
| ./node_modules/.bin/flow check --strip-root | |||
|
|
|||
| lint: | |||
| ./node_modules/.bin/eslint scripts $(SOURCES) '*.js' '**/.*.js' --format=codeframe --rulesdir="./scripts/eslint_rules" | |||
This comment has been minimized.
This comment has been minimized.
nicolo-ribaudo
Jul 4, 2018
Member
Why where these removed? I think that it was used to lint things like babel.config.js
This comment has been minimized.
This comment has been minimized.
dnalborczyk
Jul 4, 2018
•
Author
Contributor
I tried to explain it above:
- removed non-existing glob: '**/.*.js' https://eslint.org/docs/5.0.0/user-guide/migrating-to-5.0.0#-linting-nonexistent-files-from-the-command-line-is-now-a-fatal-error
- in addition removed default file .js extension glob: '*.js'
babel.config.js seems to fall through the crack though, let me check ...
This comment has been minimized.
This comment has been minimized.
nicolo-ribaudo
Jul 4, 2018
Member
I don't get why it is non-existing, it should just match everything... I guess I will download this PR and test it
This comment has been minimized.
This comment has been minimized.
dnalborczyk
Jul 4, 2018
•
Author
Contributor
oh I see. just looking at it a little closer: '**/.*.js'
I suppose there are no files with . (dot)something.js anymore. the (old?) babel.config.js probably started with a dot prior.
eslint v5 doesn't find anything starting with . (dot), and throws.
This comment has been minimized.
This comment has been minimized.
dnalborczyk
Jul 4, 2018
•
Author
Contributor
I think we could take out $(SOURCES), and replace it with '**/*.js' (everything), and .eslintignore excludes everything which needs to be excluded. that runs just fine, and includes babel.config.js as well.
This comment has been minimized.
This comment has been minimized.
nicolo-ribaudo
Jul 4, 2018
Member
Oh, i didn't notice the . in .*.js. It is ok to have sources instead of **/*.js (we would lint too much otherwise), but *.js should still be there. I tested it locally ant with this patch babel.config.js isn't linted anymore.
This comment has been minimized.
This comment has been minimized.
dnalborczyk
Jul 4, 2018
Author
Contributor
I didn't really see it either
alrighty, I'll put '*.js' back shortly and leave $(SOURCES) as is.
|
Thank you! |
…int-plugin-prettier (babel#8259) * upgrade eslint v5 (major), babel-eslint (minor), eslint-plugin-flowtype (minor), eslint-plugin-prettier (patch) * fix makefile: remove (default) .js file extension, remove non-existing glob pattern * fix linting errors * add '*.js' glob back to include babel.config.js for linting
dnalborczyk commentedJul 4, 2018
make lintandmake fixpassmigration guide:https://eslint.org/docs/5.0.0/user-guide/migrating-to-5.0.0
breaking change, v5 throws for non-existing files:`
https://eslint.org/docs/5.0.0/user-guide/migrating-to-5.0.0#-linting-nonexistent-files-from-the-command-line-is-now-a-fatal-error
-> removed non-existing glob, in addition removed default file .js extension glob
breaking change, v5 no-unused-vars reports all after-used paramshttps://eslint.org/blog/2018/04/eslint-v5.0.0-alpha.1-released#breaking-changes
-> added // eslint-disable-line no-unused-vars