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

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

Merged
merged 1 commit into from Jun 7, 2016
Merged

Conversation

nfischer
Copy link
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
Member Author

nfischer commented Jun 5, 2016

@ariporad rebased this off master

@ariporad
Copy link
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.

None yet

2 participants