chore: Create optional eslint pre-commit hook
Description
Something I was playing around with in another repository; ~~I have no experience using pre-commit lint hooks personally yet so I don't know if I find them helpful or clunky.~~ Edit: I've been running #1490 for a while now. It's nice!
This allows developers to—optionally—install a pre-commit hook that runs eslint locally ~on staged files~ before they're committed, preventing the commit if they are not in the semistandard style. This is nice for avoiding getting yelled at by the CI action.
(Traditional husky installs include a "prepare": "husky" package script, ensuring that all developers have the hook enabled. This does not.)
Testing steps
- As per the readme, run
npm run enable-hooks. - Make a change with a lint error (
let a;), attempt to commit it, and confirm that the developer is informed about the error and must fix it before continuing. - Make a change with no lint error and observe the small added delay when committing it.
- Make a change with a lint error (
let a;) on a different branch and confirm that no hooks run (I chosehuskyprimarily because it does not break in this scenario). - Run
npm run disable-hooks, make a change with a lint error (let a;), attempt to commit it, and confirm that it is no longer prevented.
Questions:
- What's the rationale behind making it optional? I feel like it would be a smoother experience to install the hook automatically.
- Why not do the full lint with
npm test?web-ext linterrors are errors on CI, so... - Could this be a precursor to us switching to the actual
semistandardpackage for true autoformatting? :D
- I know some developers don't like commit hooks! For example, the MV3 branch would have been impossible to work on in advance of web-ext being updated without bypassing hooks on each commit, and my setup doesn't have an easy way to bypass. I've seen arguments about whether they discourage frequent committing, as well, given that you still have tests run on push via CI. Didn't know what your opinion was on 'em.
- Fair; definitely an option! (I do wish it was faster. On another repo I use
concurrentlyfor multiple tasks; wouldn't be hard to set up here, though I'm not sure how well modified cli output in hooks works with some git setups.) - I don't know of a difference between using the
semistandardpackage and just running eslint!