Skip to content

chore: update jshint and move it to an npm script#454

Merged
ariporad merged 1 commit intomasterfrom
chore-linting
Jun 7, 2016
Merged

chore: update jshint and move it to an npm script#454
ariporad merged 1 commit intomasterfrom
chore-linting

Conversation

@nfischer
Copy link
Copy Markdown
Member

@nfischer nfischer commented Jun 1, 2016

I think this is worth it because:

  1. jshint is udpated, so we'll get whatever bug fixes there may have been as well as performance improvements
  2. This is now an npm package script instead of part of run-tests.js. Although cutting down on code is nice, this really shines in that it exposes linting to the commandline (so you can lint without re-running tests if you choose), and it lets jshint figure out which files to lint, instead of using common.expand() to expand the * character for Windows
  3. I removed most re-declarations of variables just because I know that's an issue for eslint, and I know we may migrate to use that in the future. This wasn't required by jshint, however
  4. I made linting a bit stricter, and actually caught some confusing parts of the code
  5. Linting now runs after tests. I think this is valuable if you want to first get your code correct, and then afterward fix it for stylistic consistency.

@nfischer
Copy link
Copy Markdown
Member Author

nfischer commented Jun 5, 2016

@ariporad rebased this off master

@ariporad
Copy link
Copy Markdown
Contributor

ariporad commented Jun 7, 2016

LGTM!

@ariporad ariporad merged commit 2e87f14 into master Jun 7, 2016
@ariporad ariporad deleted the chore-linting branch June 7, 2016 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants