Skip to content
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

add eslint 6 to peerDeps #118

Merged
merged 2 commits into from Sep 18, 2019
Merged

add eslint 6 to peerDeps #118

merged 2 commits into from Sep 18, 2019

Conversation

brettz9
Copy link
Contributor

@brettz9 brettz9 commented Jul 3, 2019

Fixes #120

@7rulnik
Copy link

7rulnik commented Aug 5, 2019

@andrea-guarino-sonarsource hey! Can we merge it? :)
Some people use npm ls on CI as a way to check that all peerDeps are installed. So, this is blocker for them to upgrade to eslint@6

@andrea-guarino-sonarsource
Copy link
Contributor

Hi @7rulnik and @brettz9 ,
Thank you for your PR.
Currently we're not working on our JavaScript analizer, but we should have a sprint pretty soon.
We'll need to go through breaking changes listed here and release a new version of eslint-plugin-sonarjs.

@brettz9
Copy link
Contributor Author

brettz9 commented Aug 19, 2019

FWIW, I've been using with eslint 6 and there don't seem to be any issues, but of course I understand the need for a review.

@7rulnik
Copy link

7rulnik commented Aug 19, 2019

Seems that we only should bump nodejs version in package.json

@vilchik-elena
Copy link
Contributor

Hi @brettz9,
All good, only one thing. Could you update min NodeJS version (>=8.x) in readme?

@brettz9
Copy link
Contributor Author

brettz9 commented Sep 13, 2019

Sure, @vilchik-elena , I can do that, but I'm not sure that you strictly speaking need to do so, since, in your package.json, engines is still showing ">=6" and peerDependencies is still showing support for older eslint versions which don't require Node 8.

Your own code could thus continue to support older Node versions (as long as users didn't update eslint to 6)--unless that is, you have made other updates to your own code base recently which in fact depend on Node 8 features.

FYI, .travis.yml is still running against Node 6 and 8 (only), so this should probably be updated as well, depending on what you want--i.e., removing Node 6 if you want to drop support, and in any case, presumably also adding tests against Node 10 and 12.

However, if you do want to drop Node 6 support (as many projects are admittedly doing), then you might consider whether you also wish to limit the eslint versions you support.

Note that if you keep Node 6 support and want to ensure Travis is testing such a version as you advertise supporting, special logic will need to be added to ensure the Node 6 environment is installing an earlier eslint version. Likewise if you want to keep support for older eslint versions and test these (even if you run them on Node 8).

@brettz9
Copy link
Contributor Author

brettz9 commented Oct 4, 2019

any chance for a release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add eslint v6.0 as peer dependency
4 participants