Setup git precommit hooks for lint issues

[joshka]

Problem

We often see action check failures due to conventional commits, cargo clippy lints, and rust format.

Solution

There's a bunch of ways to have pre-commit git hooks that check rust format, clippy, and conventional commits. Evaluate the options and choose the appropriate one to implement. I'm not sure which one is the best / most used / idiomatic.

Alternatives

None

Additional context

Installation of any required tooling should be low impact. Bonus points for borrowing the config of some reasonably stable project.

[mindoodoo]

The issue I see with git pre-commit hooks is that they aren't configured automatically when a repo is cloned, meaning that we will be relying on contributors installing the hooks, which unfortunately in my opinion is a big weakness to the pre-commit hook solution.

However, I cannot come up with another better solution off the top of my head so it is better than nothing. This stack overflow post that is seeking a solution to the issue I stated above. I like the solution put forward by the user Max Shenfield that would allow us to have git tracked hooks, which we could then facilitate the installation of with a small setup script.

Something else that could facilitate contributors following the conventional commit convention would be to include a commitizen config. That's the tool I personally use, but if there is a more widespread one I'm all ears. I can open a quick PR adding a .cz.json config file if this is deemed a suitable solution.

[orhun]

I haven't used commitizen before but it looks like a nice tool to integrate for helping with conventional commits. I agree with @mindoodoo about the disadvantages about git hooks which is they require manual configuration. However, we can go along with it unless a better solution is available.

[mindoodoo]

Also, this might be a little out of scope for this issue, but regarding the issue of enforcing conventional commit, can we prevent the squash and merge if the resulting commit name doesn't conform to the conventional commit spec ?

I'll throw together a PR real quick for commitizen.

Edit: Might not be able to do it today, but in the upcoming days 👍

[orhun]

Now that we have a commitizen config, do we still want to add git precommit hooks?

[joshka]

Could we perhaps use https://github.com/crate-ci/committed and either https://crates.io/crates/rusty-hook or https://crates.io/crates/cargo-husky instead
Why?

• They are rust based :)
• That means they can be both be installed as a dev-dependency and install themselves without the user having to do anything to run commit hooks
• That means we don't have to help committers work out npm / node issues / commitizen issues
• 🦀

This issue was put in place mainly because I wasn't sure what the right rust answer for this was. There might be other solutions out there and I was hoping someone more informed would come along and tell us.

[SLASHLogin]

While both rusty-hook and cargo-husky look very promising, since you can just add them to dev-dependencies, it looks like they're unmaintained and might not work. So they might require some work to confirm if they even work.
I'd like to suggest using something similar to the rustc's solution. They're providing the pre-push hook in the repository itself, which can be installed with their tool ./x.py setup. We could re-use cargo-make to run a script, which would do exactly the same - install the hook.

[SLASHLogin]

I've started working on it. Is it preferable for the hook to install cargo-make when detected that it doesn't exist, or it should just try to use it?

[mindoodoo]

I'd just print a suggestion to install it in the case it's not already installed, but do as you see best fit and we'll get more opinions when you open up a PR !

I'll assign you to the issue since your working on this :)

[mindoodoo]

How are you going about it in the end ?

[SLASHLogin]

I think this can be closed since #274 was merged.