Improve linting #5460

ripecosta · 2023-01-22T15:20:28Z

Improves linting configs. Runs linting on the repo.

Details:

Prettier now covers every file in the repo
On commit hook now runs both Prettier and ESLint
CI now fails on both Prettier and ESLint errors
ESLint now uses cache - runs faster locally

Note: The ESLint config currently has a blind spot on .mjs and .ts files. Fixing that blindspot is out of scope of this PR.

coveralls · 2023-01-22T15:27:47Z

coverage: 92.797%. remained the same when pulling 65463ea on ripecosta:rc/linting into ce6b591 on knex:master.

kibertoad · 2023-01-22T15:36:46Z

it's failing, workflow yamls shouldn't be modified

ripecosta · 2023-01-22T15:38:55Z

it's failing, workflow yamls shouldn't be modified

I think the issue is the use of "[[" in the npm scripts. Fix incoming.

package.json

kibertoad · 2023-01-22T16:34:26Z

package.json

    "debug:test": "mocha --inspect-brk --exit -t 0 test/all-tests-suite.js",
    "debug:tape": "node --inspect-brk test/tape/index.js",
    "coveralls": "nyc report --reporter=lcov",
-    "lint": "eslint \"lib/**/*.js\" \"test/**/*.js\" \"bin/**/*.js\"",
+    "lint": "eslint --cache --fix '**/*.js'",


lint by definition is about checking, it would be better to have explicit lint:fix command for fixing.
And CI shouldn't be running it.

Sure. But you'll end up with two different patterns for similar things. For ESLint you'll need ":fix" to make changes but not for Prettier. And for Prettier you'll need ":check" for check-only but not for ESLint.

Given the formatter and linter will always both run (in check-mode for CI and fix-mode locally) does it not make more sense to a) follow the same script naming pattern; and b) be joined the same command ?

# Conflicts: # lgtm.yml

kibertoad approved these changes Jan 22, 2023

View reviewed changes