Add git pre-commit hook to lint and test staged files

[germain-gg]

No description provided.

[jryans]

Personally, I dislike workflows that add latency to committing, so it would be nice if there's a way to opt-out somehow.

[Palid]

@jryans with husky you can use -n or --no-verify flag while commiting to ignore hooks. Would that be good enough?
Seems setting HUSKY=0 as env variable also works!

[jryans]

@jryans with husky you can use -n or --no-verify flag while commiting to ignore hooks. Would that be good enough?
Seems setting HUSKY=0 as env variable also works!

Sure, it's good enough for me at least.

I do wonder if there's some way to know how desired this actually is, so we would know if it should be opt-in vs opt-out...

[Johennes]

@vector-im/element-web how do folks feel about this proposal?

[richvdh]

pre-commit hooks are strictly opt-in anyway, because they require setup in the local git workdir. So this would amount to a recommendation for contributors rather than anything enforced.

I used a pre-commit hook in Synapse to do some linting. It was often useful. But also I often did --no-verify because I was in a rush.

So generally +1 to making it easy to set up a pre-commit hook.

[germain-gg]

Using things like husky do set up the commit hooks automatically.
And using lint-staged runs commands only against staged files, it won't be foolproof but is proven very useful for prettier checks.
I would always argue against doing a full codebase type check, but doing fast and easy linting will definitely alleviate some issues that would otherwise be caught by the slow CI

[richvdh]

husky == https://typicode.github.io/husky/, ftr

[Johennes]

We've discussed this in the EW weekly today and concluded that having linting available as a pre-commit hook would be valuable. There was no consensus on also running tests on staged files as that might blow up the hook runtime quite a bit.

To move this forward, we'd like for somebody to set up a commit hook script that triggers the linter on staged files and add documentation on how to install (and skip) it.

[Palid]

Considering that I've been a huge champion on the solution and I'm still getting notifications for it, I'll get it done today within the next few hours. 😄

[Palid]

@t3chguy @Johennes solved by:
matrix-js-sdk: matrix-org/matrix-js-sdk#3835 (1/3)
element-web: #26453 (2/3)
matrix-react-sdk: matrix-org/matrix-react-sdk#11806 (3/3)
elemen-desktop: element-hq/element-desktop#1294 (4/3)

[t3chguy]

To close this issue we'll also need to do similar for https://github.com/vector-im/element-desktop/

[t3chguy]

Thanks @Palid for kicking this off

[Palid]

@t3chguy on it!

[Palid]

Will be continued on Monday, unless someone takes over. Some stuff came out.

[Johennes]

@Palid thanks again for kick starting this. Were you still planning on wrapping up those PRs?

[Palid]

@Johennes yeah, just been busy and didn't get pinged 😅
I'll try to get this done today!