[WIP] Eslint integration #70
Conversation
.eslintrc-es6.yml
Outdated
| @@ -0,0 +1,10 @@ | |||
| env: | |||
There was a problem hiding this comment.
yaml config is super weird; can these all use .eslintrc instead?
There was a problem hiding this comment.
Yeah, I'm totally open to any format. I can change that over.
There was a problem hiding this comment.
Changed in 3cde23a
|
@wbyoung It looks like the checks are failing because the latest version of $ npm view eslint engines
> { node: '>=4' }I briefly tried to resolve this by using an earlier version of |
|
@duckontheweb the solution there is to not run the linter in every matrix build, but to explicitly include it, and only run it in See https://github.com/ljharb/is-callable/blob/master/.travis.yml#L22-L33 for an example of many travis-ci best practices. |
| @@ -0,0 +1,15 @@ | |||
| { | |||
| "env": { | |||
| "es6": "off" | |||
There was a problem hiding this comment.
why is a file named "es6" turning off the es6 env?
also, why is this a separate file at all, instead of just being in the root eslintrc?
| return installedPlugins().then(function(plugins) { | ||
| return Promise.all(plugins.map(function(plugin) { | ||
| return installedPlugins().then(function(_plugins) { | ||
| return Promise.all(_plugins.map(function(plugin) { |
There was a problem hiding this comment.
the no-underscore-dangle rule should be erroring here; can we pick a better name, like installedPlugins?
| npm.config.set('global', true); | ||
| npm.config.set('depth', 0); | ||
| return Promise.promisify(npm.commands.list)([], true); | ||
| .then(function(_npm) { |
There was a problem hiding this comment.
same throughout; let's please not add leading underscores to things :-)
| @@ -11,8 +11,8 @@ | |||
| "avn": "./bin-public/avn" | |||
| }, | |||
| "scripts": { | |||
| "test": "jshint . && jscs . && istanbul cover node_modules/.bin/_mocha --report html --", | |||
| "test-travis": "jshint . && jscs . && istanbul cover node_modules/.bin/_mocha --report lcovonly --" | |||
| "test": "eslint lib && istanbul cover node_modules/.bin/_mocha --report html --", | |||
There was a problem hiding this comment.
i'd recommend adding this in "pretest": "eslint lib". Also, eslint should run on the entire project - ie, eslint . - and .eslintignore should be used if you need to ignore linting files.
This removes
jshintandjscsas the linters and replaces them witheslint. An attempt was made to keep the rules as consistent as possible during the transition. However, there were a few areas where it seemed more appropriate to bring the code inline with the linting rules rather than adjusting the rules accordingly (see 60f82a8, 55dcbe9, and c28f3da).@wbyoung If you would like, I can turn back on some of the default
airbnbrules and bring the code base up to speed with those, but I'd like to get guidance from you first on how you would like to proceed.Addresses #67.