feat: Configure shellcheck pre-commit hook

[l0b0]

And fix reported shellcheck lints.

[jorisroovers]

Thanks for this PR!

Had considered this before, just not gotten to it. Also, I'm still debating whether to move to python entirely for task/test running. I had experimented a long time back on a separate branch using pyinvoke, but plain python might be better choice since it doesn't add an extra dependency... See https://github.com/jorisroovers/gitlint/commit/f50390ab187c1cc512037fd5b25295c3db2c59f3

Specific to this PR:

1. Let's not include pre-commit as part of this PR. That's an entirely separate topic IMO. I'd prefer to add a new ./run_tests.sh -ls flag that invokes shellcheck and then add it a step to our github CI action to invoke that: https://github.com/jorisroovers/gitlint/blob/main/.github/workflows/checks.yml
2. Tests failed on ./run_tests.sh -p, I think the PR contains a bug.
3. We need make sure we manually test all ./run_tests.sh subcommands before we merge this. Some of those are not part of CI runs.

Thanks!

[l0b0]

Had considered this before, just not gotten to it. Also, I'm still debating whether to move to python entirely for task/test running. I had experimented a long time back on a separate branch using pyinvoke, but plain python might be better choice since it doesn't add an extra dependency... See f50390a

I can thoroughly recommend testing in Python. For one thing, Python is superior to Bash for basically anything to do with testing (I've used shunit2 a fair bit and Bash quite a lot).

Specific to this PR:

1. Let's not include pre-commit as part of this PR. That's an entirely separate topic IMO.

That's a good point; removed. However:

I'd prefer to add a new ./run_tests.sh -ls flag that invokes shellcheck and then add it a step to our github CI action to invoke that: https://github.com/jorisroovers/gitlint/blob/main/.github/workflows/checks.yml

I personally don't like this solution. There's a giant shell script in front of all the test logic, when pre-commit run --all-files and pytest could do all or almost all of it.

3. We need make sure we manually test all ./run_tests.sh subcommands before we merge this. Some of those are not part of CI runs.

Why not? I ran ./run_tests.sh --all, but a bunch of those tests fail even on the main branch, so I've no idea whether my changes broke things.

[l0b0]

Ping?

[l0b0]

Rebased.

[jorisroovers]

So I still want to move away from bash/shell and move to pure python for checks and tests. Perhaps moving to hatch, tox or something else still. I plan to look into this before the end of the calendar year.

Most or all of run_tests.sh would go away as a result. With the limited time I have for gitlint, I don't think investing it in this PR is then the best thing. Appreciate the effort you put in here though, thanks!

[l0b0]

No worries. Would you be interested in other pre-commit hooks and/or automated linting?

[jorisroovers]

For me, CI and the move away from run_tests.sh kind of go hand in hand and as such I prefer to wait a little bit. There's definitely a few interesting ones (some I've used before) in what you linked. Today we don't use pre-commit, that might change - allow me some time to consider this :-)