To git hooks, or not to git hooks: that is the question

Hi all,

as per title, I am considering the possibility to add git hooks to a Phoenix project.
We are a team af a few people (< 10) with the same editor setup.
No CI workflow is present at the moment.

That is why I would like to add git hooks, do you have advices on which checks to run and when?

I would love for them to don’t get much in the way (for example, when prototyping features on temporary branches).

I thought about this:

  • mix clean && mix compile on push
  • mix format on commit (is that a good idea to format files in place on commit? Running --check-formatted could be too annoying)
  • mix credo and mix test as responsibilities of developers

I’d love to hear from you
Kind Regards

2 Likes

Start with solving that.

IMHO commit hook should not modify code.

7 Likes

You might want to look at ex_check | Hex.
You could put that on your hook.

2 Likes

I’m curious about the motivation of this step; why would somebody be committing code that doesn’t compile? If it doesn’t compile, it can’t run in tests or even with manual prodding.

2 Likes

Hey! There is a cool lib for that - helps maintain the proper “local CI” (Git hooks) for all of the devs in the project: GitHub - qgadrian/elixir_git_hooks: 🪝 Add git hooks to Elixir projects.

2 Likes

FYI this will be a real pain the first time somebody on your team needs to push up code that won’t compile so that a team member can help them.

I agree with @hauleth, this is the problem to solve. You can put specific rules about specific branches, rules about how PRs get merged etc, and it lets you handle mix credo and mix test centrally too.

7 Likes

In our setup you have to run tests manually. If you forget to run them you could very much commit code that doesn’t compile :confused:

Which is the reason I thought about git hooks

1 Like

Thank you all,
setting up a CI seems the most logical first step to make.

1 Like

I have a shell script in every project I work on called bin/dev/shipit.

It does all the things such as:

  • runs tests
  • runs dialyzer
  • runs mix audit
  • runs the formatter

If all those things pass, then it pushes. It’s very simple but it works (if everyone remembers to run it instead of just git push). If you for some reason need to push with one or more of those steps failing, you can just not run the script but instead just run git push.

1 Like

I am strongly against imposing workflows on developers collaborating on a project. Everybody has a different order/process for producing code that is ready for review, and until its ready for review (and CI) all barriers and challenges should be minimized. If part of your process is to autoformat your code on every commit using a git hook, by all means do that, but please, please don’t impose that on your fellow devs who do not want to think about formatting at all until everything is ready.

6 Likes

I like writing a custom mix ci task/alias. The CI server should be configured to execute that, but also each developer can choose to run that on their own machine in case they need help diagnosing a problem that only occurs on CI.

4 Likes

Hard :-1: on hook. Fail on CI to enforce what you want

4 Likes

That is a valid point.
Didn’t think about that because we are a few and with the same setup.
I would agree with you.

I like very much this approach. Thank you.

I agree with this. My approach is simply just to have our CI pipeline runs mix format --check-formatted.
It is then up to the individual developer to choose how to ensure that all the code they push is formatted. Most of our devs use VS Code and have “format on save” configured, one dev uses vim and simply does mix format && git add -p whenever they’re setting up a commit.

Where are you hosting your code? It can be very simple to set up CI (at least a basic one that checks formatting and runs tests), and I can probably suggest a simple way of doing it based on where your code lives.