Skip to content

Conversation

@compwright
Copy link

@compwright compwright commented Jul 27, 2018

jshint is no longer actively maintained, and has security vulnerabilities.

Since eslint requires Node.js 6+, this PR drops support for Node.js 4-5.

@thomseddon
Copy link
Member

Thanks, this has been fixed in #511

@thomseddon thomseddon closed this Aug 3, 2018
@compwright
Copy link
Author

@thomseddon I'm glad you're working on this, but I'm puzzled why you declined mine, especially since mine fixes all the security issues that npm audit warns about, and #511 does not (yet). I've left feedback on that PR where it could be improved.

@thomseddon
Copy link
Member

Hmm, maybe I was too rash with this! I hand't noticed that jslint was dead and assumed all the line changes were superfluous - now I see they were actually lint fixes! Which makes this even more awesome.

Will close #511 in preference for this

@thomseddon thomseddon reopened this Aug 3, 2018
@thomseddon thomseddon mentioned this pull request Aug 3, 2018
@thomseddon
Copy link
Member

Only issue I can see is that eslint doesn't seem work with node 4.

Is there any way we can keep support?

@compwright
Copy link
Author

@thomseddon IMO, it's time to move on. Node 4 is EOL. Perhaps this should be released as 3.1 instead of 3.0.1. I'll update engines.

@mjsalinger
Copy link
Contributor

@thomseddon I think we should drop support for node 4. Node 4 is EOL and no longer supported by node core. In general we should support the versions that are considered active by node.js - so currently, it's 6, 8, and 10..

@thomseddon
Copy link
Member

👍 to drop support for 4, all looks good LGTM

@mjsalinger
Copy link
Contributor

A point was made in #496 that we should merge non-breaking changes into 3.1.0 (especially since we're dropping support for 4.x) before making a non-breaking release. Can we refactor this PR to support 4.x for now? We'll drop support for 4.x very soon.

@mjsalinger
Copy link
Contributor

@compwright Can you rebase your changes against the dev branch and fix conflicts? I'd like to merge the eslint change into 4.x..

@mjsalinger mjsalinger added this to the 4.0.0 milestone Aug 27, 2018
@compwright compwright force-pushed the upgrade-deps branch 5 times, most recently from be1632b to b88fcf8 Compare August 27, 2018 16:49
jshint is no longer maintained, and has security vulnerabilities that have not been addressed.

eslint is only supported on Node 6+
@compwright compwright changed the title Upgrade dependencies to fix security vulnerabilities Switch to eslint Aug 27, 2018
@compwright
Copy link
Author

@mjsalinger good to go! Thanks

Copy link
Contributor

@mjsalinger mjsalinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mjsalinger mjsalinger merged commit ff09b2b into oauthjs:dev Aug 27, 2018
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.

3 participants