[#4] Set up the standard linter on CI #5
Conversation
| "webpack": "^1.12.2" | ||
| }, | ||
| "standard": { | ||
| "env": [ |
There was a problem hiding this comment.
Note that this applies the mocha env to all files being linted, including source (non-test) files. If we'd prefer, we can switch to using comments at the head of files
There was a problem hiding this comment.
I think it's ok, I doubt that describe, it, etc. would ever leak to the source code, so it's not a big deal.
| @@ -26,24 +26,27 @@ describe('integration tests', function() { | |||
| publicPath: '/bundles' | |||
| } | |||
| }) | |||
| compiler.run(function(err, stats) { | |||
| compiler.run(function (err, stats) { | |||
| if (err) { | |||
There was a problem hiding this comment.
Standard style requires all errors be handled. I rethrow here, which will fail the tests, but I could instead call assert.fail if we prefer.
| "test-ci": "env NODE_ENV=test mocha --require ./test/power_assert_loader.js test/basic.js test/integration.js" | ||
| "lint": "standard", | ||
| "lint-fix": "standard --fix", | ||
| "test": "npm run lint && env NODE_ENV=test mocha --require ./test/power_assert_loader.js test/basic.js test/integration.js --watch", |
There was a problem hiding this comment.
This will only run the linter one time, and then watch the unit tests. If we'd prefer, I can set up chokidar to run both tasks when files change.
There was a problem hiding this comment.
Hm, maybe it's better to create separate lint-watch task? There is an amazing https://www.npmjs.com/package/eslint-watch that does this job well.
As a side note. I think that the previous task naming is unfortunate. If I (or let's say CI) run npm test I expect to run it in a single mode. Maybe we should move watch mode to npm run test-watch for consistency sake?
| @@ -26,24 +26,27 @@ describe('integration tests', function() { | |||
| publicPath: '/bundles' | |||
| } | |||
| }) | |||
| compiler.run(function(err, stats) { | |||
| compiler.run(function (err, stats) { | |||
| if (err) { | |||
| "test-ci": "env NODE_ENV=test mocha --require ./test/power_assert_loader.js test/basic.js test/integration.js" | ||
| "lint": "standard", | ||
| "lint-fix": "standard --fix", | ||
| "test": "npm run lint && env NODE_ENV=test mocha --require ./test/power_assert_loader.js test/basic.js test/integration.js --watch", |
There was a problem hiding this comment.
Hm, maybe it's better to create separate lint-watch task? There is an amazing https://www.npmjs.com/package/eslint-watch that does this job well.
As a side note. I think that the previous task naming is unfortunate. If I (or let's say CI) run npm test I expect to run it in a single mode. Maybe we should move watch mode to npm run test-watch for consistency sake?
| "webpack": "^1.12.2" | ||
| }, | ||
| "standard": { | ||
| "env": [ |
There was a problem hiding this comment.
I think it's ok, I doubt that describe, it, etc. would ever leak to the source code, so it's not a big deal.
This resolves #4