-
Notifications
You must be signed in to change notification settings - Fork 5
Fix all eslint issues #5
Conversation
Smarker
left a comment
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.
Looks good. I see that I will have to make changes in my code since I might have used double quotes a couple of times rather than single quotes.
| node_js | ||
|
|
||
| node_js: | ||
| - "7" |
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.
package.json has node version 6.0.0 not sure if we want to keep our version at 7 or change what is in .travis.yml from 7 to 6.
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.
Good catch. We can run on both actually, see 0aa6524.
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.
NB: you can use ./node_modules/.bin/eslint src --fix to auto-fix simple issues like double-quotes vs single-quotes.
erikschlegel
left a comment
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.
Thank you!
This pull request fixes all eslint issues and sets up CI so that we always run tests and lint on every check-in/pull-request.
The pull request also fixes a few bugs uncovered by eslint, mostly to do with referencing undefined variables.