Upgrade to eslint 4 #4738
Upgrade to eslint 4 #4738
Conversation
|
Do we really want to upgrade to ESLint 4 if it's bugged and forces us a syntax we don't want? what are the benefits? |
|
ESLint 4 found a good number of real issues as well. I fixed all those in a separate commit earlier: f6b6956 It's only this one place in the code base that there was a regression in behavior. I think all the real issues it found may outweigh having to add parens in one spot |
benmccann
force-pushed
the
eslint4
branch
2 times, most recently
from
b34b2aa
to
7c9d0cc
Compare
|
I've updated this to use the newest eslint with a fix for the bug I reported. It now passes locally without needing the extra set of parens It's still failing on codeclimate. It must not be using the latest version of eslint. I've sent a PR to upgrade codeclimate: codeclimate/codeclimate-eslint#326 |
benmccann
force-pushed
the
eslint4
branch
2 times, most recently
from
57369a6
to
633f099
Compare
|
@simonbrunel I now have eslint 4 working both locally and on codeclimate without any code formatting changes. You should be able to merge this PR now. Thanks for the review and feedback |
|
@etimberg @simonbrunel would we be able to merge this now? Even if we want to have a 2.7.2 I think this PR should still be okay |
|
I am ok with this |
I had to add one extra set of parentheses. I filed as an eslint bug for that one: eslint/eslint#9285
Otherwise I fixed all the issues in past PRs