pre-commit: Switch to GitHub action; add eslint fixer

[pokey]

• Partially addresses Cleanup Typescript imports in pre-commit #640
• Supercedes pre-commit: Keep prettier version in sync; add eslint fixer #1400

Checklist

• I have added tests
• I have updated the docs and cheatsheet
• I have not broken the cheatsheet
• Disable pre-commit.ci
• Disable pre-commit.ci lite
• Change branch protection rules
• Check boxes in Cleanup Typescript imports in pre-commit #640

[pokey]

Huh pre-commit.ci lite unfortunately doesn't seem to work. I tried changing a lint rule to see if it would auto-fix, but nothing happens 😕

edit: Ok it did work, but man was it slow 😅

[pokey]

Ok I switched the push action to https://github.com/stefanzweifel/git-auto-commit-action instead of pre-commit.ci lite, because the former is basically instant, whereas the latter seems to take a while

[auscompgeek]

Does this work for forks?

[pokey]

Good question. I'll give it a whirl

[pokey]

Haven't tested yet, but looks like a bit of a hornet's nest and potentially introduces security vulnerability to get it to work 😬 stefanzweifel/git-auto-commit-action#211

So do we switch to pre-commit lite and deal with really slow commit, or switch to the setup from my other attempt with syncing versions? Downside of the latter is we can't easily run custom fixers like if we wanted to autoformat our recorded tests using our custom yaml writer

[auscompgeek]

Hmm. It might be possible to get something working with the completion of this workflow triggering another workflow that runs in the main context? That'd be super complex though, and sounds like even more of a nightmare.

I've seen workflows that use a personal access token to get this sort of thing to work, but that's probably just as risky.

[cursorless-bot]

Confirmed that this setup doesn't work for forks. See https://github.com/cursorless-dev/cursorless/actions/runs/4658630193/jobs/8244534702?pr=1409:

remote: Permission to cursorless-bot/cursorless-fork-test-2.git denied to github-actions[bot].
fatal: unable to access 'https://github.com/cursorless-bot/cursorless-fork-test-2/': The requested URL returned error: 403

Going to take another swing at pre-commit.ci lite; maybe the slowness was just a one-off 🤷‍♂️

[pokey]

Ok I switched back to pre-commit.ci lite; was actually quite fast this time around, so I'd be tempted to just ship it. Being able to run our own custom fixers using node scripts etc I think is worth the price of slower CI fixes

[pokey]

I do wonder why GitHub's security policy doesn't allow GitHub actions to push from the context of the forked repo to a branch on that repo 🤔

[auscompgeek]

FWIW we don't have to abandon pre-commit.ci entirely, we can skip eslint/prettier on pre-commit.ci and just run eslint and prettier on GitHub Actions

[pokey]

FWIW we don't have to abandon pre-commit.ci entirely, we can skip eslint/prettier on pre-commit.ci and just run eslint and prettier on GitHub Actions

What's the benefit of having both systems active?

[auscompgeek]

🤷

There's a potential cache speed benefit with pre-commit.ci, but I guess we don't know exactly how much caching will help us here since the cache doesn't exist on main for the workflow to use yet.

[pokey]

🤷

There's a potential cache speed benefit with pre-commit.ci, but I guess we don't know exactly how much caching will help us here since the cache doesn't exist on main for the workflow to use yet.

pre-commit.ci is significantly faster, as you suspect, but tbh most of the fixes come from prettier, so I'm not sure the benefit of putting the other fixers in a different CI system justify the complexity / races of running two systems