Pre-commit Hooks Are Great and Here Is Why Your Team Must Adopt Them

Pre-commit Hooks Are Great and Here Is Why Your Team Must Adopt Them

I recently stumbled twice upon discussion about pre-commit hooks. Both times it was to tell people how bad pre-commit hooks are and how they shouldn’t be used.

I don’t agree with this statement and I want to explore why pre-commit are good and where the concern from both create comes from.

What Started This Whole Conversation

This all started with Theo Ping (his YouTube channel), who had a very strong opinion against pre-commit hooks. In a very short video, Theo says that pre-commit are bad and that they are a statement on how bad the CI of companies using pre-commit hooks are.

Here is the video.

Classical Theo!

The other option on pre-commit hooks comes from Cory House, where he defines pre-commits hooks as a waste of time.

Here is the tweet.

https://twitter.com/housecor/status/1612804261821259776

Those two options made me think about pre-commit hooks. I currently work at Proton and the frontend code uses pre-commit hooks to run linting on staged files.

You can find the monorepo of the clients here:

GitHub - ProtonMail/WebClients: Monorepo hosting the proton web clients
Monorepo hosting the proton web clients. Contribute to ProtonMail/WebClients development by creating an account on GitHub.

What Is Wrong With Pre-commit Hooks

Two main points were raised against pre-commit hooks. Here is the quick summary of the main points.

First, pre-commit hooks are a productivity nightmare. They block developers from doing their job. This impact throughput of the team.

Besides that, according to both Theo and Cory, quality check must be done on the CI and not in the developers commits. Developers should be able to write code, commit it instantly and has feedback coming from the server.

Those two points implies that a developer will lose significant time if the code is checked at each commit. Besides that, it’s not a developer concern to write correct code, it only becomes its business once the pipeline breaks.

What Might Cause the Critique

I tried to understand why both Theo and Cory don’t like pre-commit hooks. Both seem to have had bad experience with pre-commits that took way too long to execute.

Cory stated that some projects he worked on had pre-commit that took up to 30 seconds. I’m the first to agree that pre-commit hooks that take 30 seconds are bad.

When it comes to Theo, he says that he likes to do a lot of commits when developing and squash them (if asked in the PR) to keep a cleaner git history.

Why Pre-commit Hooks Make Sense

In my opinion, pre-commit hooks are good and are a great way to ensure that the code that hits your VCS is clean and properly formatted. There are four main reasons that come to mind where pre-commits hooks are great.

Clean Code Sent to Server

The first advantage I see when thinking of pre-commit hooks is that the code that hits the server is always well formatted. There aren’t any poorly formatted code that hits the VCS.

Having poorly formatted code isn’t that bad since it’s possible to squash commits, but it’s still great since developers won’t forget about something somewhere in their code.

Reduce the Number of Commits

I get that doing a lot of small commits is great and helps organize. However, I don’t think that having to commit small changes due to limiting the issue is productive or helps productivity.

Besides that there is the squashing commits that, even if they don’t take much time, could still be avoided by having pre-commits hooks. No need for a developer to leave a “please do a squash” comment on a merge request.

This is a time saver for both the developer and the reviewer that will often have to validate the merge request once the squashing is done.

Reduce Context Switching

Let’s say you’re working on a feature. You commit and push your code and go to the next task. You start working on it and after 3 minutes you get an email saying that a pipeline is failed.

Context switching is bad, and developers should be able to get in “the flow” to be more productive (as it’s the case for many other professions). Having to look at an email, checkout to your old branch, fix the issue and push the changes is not ideal.

This context switching will impact the developer’s productivity, and pre-commits hooks will avoid this. The sooner the feedback, the cheaper it is to fix.

Takes Less Energy

Lastly, there is an elephant in the room that neither Theo nor Cory addressed. Both agree that CI is expensive and that running dozens of pipelines just to tell developers to delete an unused import is a waste.

However, CI is not only expense but also energy hungry. Having servers running at full speed to make sure that developers formatted their code correctly is a waste.

I’m not saying that CI shouldn’t run a lint test. I definitely should. What I’m saying is that CI should have thousands of failed pipelines because of an unused import or variable.

This is pure waste since pre-commit hooks could have prevented that.

Subscribe to the newsletter to get new articles right in your inbox!

Subscribe to the newsletter

Pre-Commits Hooks Aren’t Perfect

As everything in life, pre-commit hooks are not the perfect solution. Of course, they can ensure some code consistency and, in my opinion, are great for productivity.

However, this is true as long as they are done well. When done poorly, as it seems it was the case for Cory and Theo, they will make developer life miserable.

Pre-Commit Must Be Fast

That’s the main complaint Cory had again, pre-commit hooks. If a pre-commit hook takes longer than 10 seconds, the pre-commit hook is too long. The value can change depending on your sensitivity, but I feel like 10 seconds are bearable.

First, only do the lint check on the file that was changed. At Proton, all the frontends are in a single monorepo. Doing a lint check of the whole repo takes ages.

This is why I would recommend checking lint staged, which will only run the lint check on the stage’s files.

This way, small commits will result in quick pre-commit hooks.

GitHub - okonet/lint-staged: 🚫💩 — Run linters on git staged files
🚫💩 — Run linters on git staged files. Contribute to okonet/lint-staged development by creating an account on GitHub.

At my job, the pre-commit hook takes about 4 seconds at each commit. I had instances where it took longer, but this was because I made some changes to some configuration files, which isn’t something you do regularly.

No Test or Only Affected Tests

Tests takes time and I agree that pre-commits hooks are not the place where you want to run them. A developer writing tests on part of a project will run the tests and knows that they pass.

Tests are here as a safeguard to make sure that broken code cannot reach the main branch. I won’t recommend running tests in pre-commit hooks since it will make the whole commit process way too long.

Let’s say you still want to run tests during pre-commit. Please only run tests on affected files and not the whole project.

The following article will go into more details on how to do this kind of implementation.

How to use jest and lint-staged to detect only files that have been changed
How do we run only the unit tests for files that have changed before git commit? As we move forward with EPC, unit testing is a necessary skill, and it’s important to run single tests before local Git commits, as you can’t put all the pressure of single tests on the pipeline. After all, it costs a l…

Use It on Team Projects Only

I think that it goes without saying that pre-commit hooks are really useful on team projects. You don’t want to waste other people’s time with poorly formatted code or lint errors.

Yes, you can have pre-commit hooks on your personal project. There is nobody stopping you. However, I feel like doing a sanity check from time to time is already plenty sufficient.

Look at the build logs and see what are the warnings. You could also take the time when doing updates to run prettier and make a lint check in case something slipped through.

The Process I Follow to Avoid Context Switching

I said before that pre-commit hooks are great to avoid context switching, and I stand by it.

Knowing right before a commit that the code, you want to commit isn’t correct is something important, in my opinion, since you won’t have to come back to the code to fix small mistakes in the future.

There is nothing more frustrating than receiving an email saying that you have an unused variable in a file 5 minutes after you pushed your code. You were already working on something else, and you have to go back to the branch.

I try to make sure that the code I push is clean and that tests are passing, so I have the peace of mind to know that I did my best.

Here is the process I follow when working on a feature or a bug fix:

  • I create commit with small changes, so I can review them and make sure the code I commit is correct. I won’t push code unless I required.
  • While working on the feature, I will either amend the commit or create a new commit if this makes sense. Still no push.
  • Optionally, I would run the tests on the part of the code I’m working on if I feel the need. I often run the code while I review a commit, so I won’t waste time.
  • Optionally, I would squash my commits if I feel the need. Still before pushing.
  • Once I did all those steps, I will push the code.

Following those steps isn’t the perfect solution. There are times when I’m in a hurry and I have to push core more frequently. There are also times when I break a CI pipeline because of a mistake I made.

I don’t think it’s possible to have a perfect process that guarantees that nothing will break. However, there are ways to reduce context switching and reduce the amount of back and forth between developers.


Wrap up

As you can see, there are many reasons to love pre-commit hooks and many reasons to have some in your project.

I would argue that waiting 4 seconds at every commit isn’t a big deal and won’t hurt your productivity. You can go back to your code while the pre-commit hook is running, you don’t have to wait in front of it.

There are many things that can reduce developer productivity besides pre-commits. Design changes, poorly thought out or adapted features, or the unexpected limitation of a tool/service are some of the many reasons that waste developers’ time.

It is, in my opinion, that’s those kinds of events that need to be avoided if we want to improve developer productivity, the time it takes to commit code isn’t the issue.