-
Notifications
You must be signed in to change notification settings - Fork 145
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
Move from JSHint to ESLint #145
Conversation
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've split the test run from the other tasks, this speeds up the performance of the runners a little. This is also needed, as ESLint doesn't support all the Node.js versions we have on CI.
@@ -178,12 +178,12 @@ git clone https://github.com/puleos/object-hash | |||
If you want to stand this up in a docker container, you should take at look | |||
at the [![node-object-hash](https://github.com/bean5/node-object-hash)](https://github.com/bean5/node-object-hash) project. | |||
|
|||
### gulp tasks | |||
### Package scripts |
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'll be moving more of these scripts over to the package, rather than running everything through Gulp. Once this work is done it will simplify the setup a little, so there is no dependency on Gulp any longer.
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.
👍
It would be nice to remove the dependency on Gulp.
@@ -31,6 +31,6 @@ describe('replacer option', function() { | |||
return k; | |||
}; | |||
|
|||
hash.writeToStream({foo: 'bar'}, {excludeValues: true}, strm); | |||
hash.writeToStream({foo: 'bar', replacer: replacer}, {excludeValues: true}, strm); |
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 is a fix for the tests. The replacer
variable was not used, which was previously not detected, as JSHint wasn't running on the test files.
Moves from JSHint (which has been unmaintained for some time), to ESLint. The configuration now extends the recommended preset from ESLint, and enables some equivalent rules that were previously enabled with JHint.
The linter will now also run on all files in the project, not just
index.js
. It also looks like some issues slipped through the cracks with JSHint that I've also gone ahead and fixed.