Add linter

[ndhoule]

@jbnicolai @boneskull

Addresses #1359

The Make task uses eslint's --reset flag to disable all default ESLint settings because ESLint is planning on making that the default in the future and I'd rather not have to go back through and change all the settings when that happens. I know there are a lot of settings here, but would be good to get some feedback on what's here. Most of the rules are pretty uncontroversial and we already follow them, but there are a few sections (labeled in .eslintrc) worth looking at.

We should also add more directories (e.g. bin and test), but I'm thinking we can do that in a separate PR to speed this one along to minimize merge conflicts. (Happy to handle that too.)

[ndhoule]

Kinda screwed up here and just deleted any rule I wanted to disable; in reality we should leave them in the config. --reset is not yet the default, so most editors' linting plugins will re-enable those rules. Will add these back in before merge.

[ndhoule]

once we settle on a final rule list I'll remove those comments in .eslintrc

[boneskull]

if we're building with browserify, we can just use the node env

[ndhoule]

Mostly to pick up on document, window, etc. in the reporters, lib/utils.js, and a few other files. We should probably remove it eventually though

[boneskull]

in those cases we could do a

/* eslint-env browser */
// ^ top of file

[ndhoule]

👍

everything in {lib,test}/browser should have its env set to browser, maybe a few reporters. I think the references to browser stuff elsewhere is feature detection. was being lazy but I'll just fix those

[boneskull]

@ndhoule thx for all the work on this

[ndhoule]

@boneskull Think I addressed all your comments (added your config, converted .eslintrcs to YAML, addressed notes like removing browser: true, etc). Disabled some of the rules that were making linting output particularly noisy; I think it's important to minimize linting noise because people will end up ignoring the linter otherwise. Will take care of some of those later (or other folks are welcome to tackle them :P).

Just rebased, should be good to go—let me know if you want to see other changes

[ndhoule]

Only point to updating npm was to fix 0.8 CI builds, so amended it PR to only update npm when necessary. Also added npm >= 1.3.7 (introduction of caret) to Mocha's engines field.

[ndhoule]

just pinging here. I think I've addressed all issues, let me know if that's not the case

[boneskull]

taking another look now. I'm probably going to merge this unless someone objects.

[boneskull]

@ndhoule The only thing I see that would prevent me from merging as-is is the npm change. Let me check on package.json really quicklike

[boneskull]

@ndhoule OK, so that's necessary because why? If we changed the caret in the eslint version to a tilde, would the 0.8 Travis run work?

[boneskull]

If we can avoid that, it'd be great, otherwise this would have to be rebased into branch v3.0.0

[dasilvacontin]

~ matches only new patch versions. If we plan on manually updating the dependencies, I'd say it's fine leaving the tilde. Otherwise we might miss some bugfixes due to some new feature release.

And yeah, using ~ makes updating npm unnecessary for travis.

[boneskull]

well, I know that, I just wasn't sure if the caret was there for a reason

[dasilvacontin]

yeah, sorry, too early in the morning here :x

[ndhoule]

@boneskull

@ndhoule OK, so that's necessary because why? If we changed the caret in the eslint version to a tilde, would the 0.8 Travis run work?

We can change the caret to tildes, which should leave Mocha installable on npm > 1.3.7, so long as users aren't trying to install it with its dev dependencies.

ESLint has carets in its dev dependencies, so including it will make npm 1.3.7+ a hard dev dependency for Mocha going forward—so those lines in .travis.yml will still be necessary.

[ndhoule]

@boneskull rebased from master and fixed the caret issue.

[boneskull]

ty. That's fine with the dev deps.

[ndhoule]

@boneskull Cool. Removed npm from the engines field, since removing the caret made that not a req. (I had added it in when I added the linter.) This should be good to go, then.

[boneskull]

gonna try to merge this.

[boneskull]

just waiting on CI. oh boy oh boy oh boy

[boneskull]

@ndhoule thanks a ton!