feat: add linting job to CI workflow and simplify test script #158
Conversation
| "pretest": "[ \"$NODE_LTS_LATEST\" != \"\" ] && [ \"$MATRIX_NODE_VERSION\" != \"$NODE_LTS_LATEST\" ] && echo 'Skipping linting' || npm run lint", | ||
| "test": "npm run tests-only", | ||
| "tests-only": "tap", | ||
| "test": "tap", |
There was a problem hiding this comment.
npm test should always run everything. you run npm run tests-only if you only want to run the tests.
| lint: | ||
| name: Lint | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 'lts/*' | ||
|
|
||
| - name: Install dependencies | ||
| run: npm install --ignore-scripts --include=dev | ||
|
|
||
| - name: Run lint | ||
| run: npm run lint |
There was a problem hiding this comment.
i believe we already run the linter via pretest
There was a problem hiding this comment.
So, should the linter run after the tests using posttest? I've never agreed that the linter should run with the tests—it should be something separate and in a different job.
There was a problem hiding this comment.
Locally, everything should run with npm test.
The CI jobs should be configured so that the linter runs once, and the tests run on every supported node version (minor or major).
Thus, npm test runs everything, and CI workflows run more granular scripts.
There was a problem hiding this comment.
I don't think the linter should live under the test command.
There was a problem hiding this comment.
Why not? That's how every project I've maintained or contributed to works.
The linter is not the tests. I want to run the tests peacefully without worrying about the linter. I can worry about the linter later after finishing my changes.