Use pre-commit and pre-commit CI

[JesperDramsch]

As I have just tried to open a PR with #1334 I see that there are two Actions running on pull_request_target. And I'm getting errors for scripts that I never touched and no straight-forward way to fix them, except for modifying out-of-scope files.

On many Python projects, I have been using pre-commit hooks and added the pre-commit ci, which is free for open source projects.

shfmt is available as a pre-commit hook here:
https://github.com/pecigonzalo/pre-commit-shfmt

Four main reasons:

• This gives users a straight-forward way to ensure their formatting / code-style is consistent with the project expectations.
• The installation is very easy
• The CI can automatically fix easier formatting errors on pull requests
• The hooks are only run on files that were changed.

This reduces the burden on both contributors and maintainers.
(It can be extended to more hooks as well without having to worry about setting up Go correctly etc.)

[michelroegl-brunner]

@se-bastiaan

[se-bastiaan]

Seems that the PR workflow still has some issues then, probably because it is the diff between the merge commit and the PR base. And the merge commit also contains changes from main. Still need to make a little change there then so that it uses the PR commit instead. As I've still been doing this week, since it is quite hard to test every single type of change. I would ignore those issues for now, a lot of scripts still have formatting issues.

My idea with the PR workflows was that anything that is not correct is posted as comment (that's why we need pull_request_target). That way anyone, even if they're not familiar with Github actions, will be notified of changes that are not accepted. And nobody needs to jump into the GHA detail pages.

I do agree that pre-commit would be nice to use, mostly if pre-commit.ci is added to the mix. For local dev environment it requires people to set it up with both Python and Docker (because that's what the hook uses). If the CI part would take care of that (the formatting) it would be a win especially for anyone who's less familiar with these tools. I think we would currently be mostly leaning on the shfmt plugins for IDEs, at least that's how I read #920.

[JesperDramsch]

Yeah, Github actions are tricky. Spent way to much of my time fiddling with those.

You probably know this anyways, but careful running code on pull_request_target does potentially expose all Github secrets. We used to handle this with two-stage actions. One with pull_request that does the analysis / tests code, stores the results as an artefact and another with pull_request_target that updates any comment (or other sensitive information).

I think the pre-commit action can be switched to either use the system binary, docker, or Go. But having to set up Python to use it, is of course an annoyance. Just as a side-note however, that installing pre-commit hooks is optional in a sense. So people can use them to avoid surprises, but as long as shfmt is ran the CI will likely be fine.

Additionally, one could use ShellCheck to catch bugs before they arise:
https://github.com/koalaman/shellcheck-precommit

I should note that I'm not that invested in it. I just found it to be a very useful tool. I don't plan to work on this project more than the occasional bugfix when I see a script break.