wbyoung / avn

Automatic Version Switching for Node
MIT License
1.14k stars 54 forks source link

[WIP] Eslint integration #70

Open duckontheweb opened 6 years ago

duckontheweb commented 6 years ago

This removes jshint and jscs as the linters and replaces them with eslint. 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 airbnb rules 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.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling c28f3da53b321316ff956afb262bfce82982cf32 on duckontheweb:eslint into 498ee492c727839ce463b5c4c63935110bf5293f on wbyoung:master.

duckontheweb commented 6 years ago

@wbyoung It looks like the checks are failing because the latest version of eslint requires node >= v4 and the TravisCI build uses v0.12.7:

$ npm view eslint engines
> { node: '>=4' }

I briefly tried to resolve this by using an earlier version of eslint. The problem is that the eslint-config-airbnb* packages have a slew of dependencies and we would need to be very specific in which ones we pulled. I can do this if you'd like, but I'm wondering if it would be easier to upgrade the Travis Node version to one of the LTS version above v4.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling f55a721020f193c4f3f38eebdae8890ea5517f06 on duckontheweb:eslint into 498ee492c727839ce463b5c4c63935110bf5293f on wbyoung:master.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 3cde23a9aed58a66b28d2b75e1febf033fa83a57 on duckontheweb:eslint into 498ee492c727839ce463b5c4c63935110bf5293f on wbyoung:master.

ljharb commented 6 years ago

@duckontheweb the solution there is to not run the linter in every matrix build, but to explicitly include it, and only run it in lts/*. There's no reason to run the linter more than once :-)

See https://github.com/ljharb/is-callable/blob/master/.travis.yml#L22-L33 for an example of many travis-ci best practices.