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
Update: Eslint to 3.0 and update CI builds (fixes #4638) #4680
Conversation
awesome @gyandeeps! that's interesting because https://github.com/babel/babel/blob/master/packages/babel-cli/src/babel-doctor/rules/deduped.js is using async functions? dono if the rule is wrong or something config is https://github.com/babel/eslint-config-babel/blob/master/index.js |
i'd be ok with just turning off the rule for those files/lines - made #4678 to remove it anyway. But yeah should investigate why it's saying that for async functions (possibly just babel-eslint issue as well) |
how about for this PR, i just turn this rule off inline for those 2 cases? |
Yep that's fine! |
Done. its a babel-eslint issue, it sets the |
Weird since it shouldn't be doing that anymore babel/babel-eslint@1823b5e |
script: make test-ci | ||
script: | ||
- 'if [ -n "${LINT-}" ]; then make lint ; fi' | ||
- 'make test-ci' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will run the tests even in the linter run - i think you want if [ -z "${LINT-}" ]
here to skip this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do this. This way it will be fast as that build will only do lint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we can skip npm install
in that case (not that it would matter much)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope, you need it for eslint
awesome stuff! π |
Ok I was investigating and it's because of βββ¬ eslint-config-babel@1.0.1 So it's still using the old babel-eslint |
Update ESLint to 3.0 and make sure lint is only run under nodejs version greater than 4 (run on latest).
Questions:
Lint has 2 errors.
@hzoo any idea what should i do with these errors? (asking bcz I dont have a good idea abt the code base yet π )