Here's a little rant from a technical owner perspective.
I've seen this at various places. A senior developer sets up a repository, everything is well and all right, the main branch is protected and there are CICD pipelines set up. Everything is fine. And then, with a little thought given to it, pre-commit git hooks are set up.
Obviously, I mean the hooks that run on your very machine, before you can actually commit stuff.
Is it a good idea? Maybe it's a bad and micro-manager-like way to power flex? Well, let's see.
The good intentions
I get it. This option looks attractive on the surface. Here are points that look all right at the first glance.
Git history looks cleaner
This point is stupid because you will squash pull requests anyway so any weird commits not passing tests are removed.
Save time on CI
Fair point, you will save some time, but are you really that tight on budget that you can't spare some seconds?
If this is a problem, maybe your pipelines can be configured better. Follow the fail-early method, run linter first, then units, then integration, and so on.
Ultimately, the developer working time is much more expensive, and with pre-commit hooks you lose a bit of this time.
Force developers to test before pushing
This point is distrust in your workers, just disguised. If you don't trust your developers, fire them, or fire yourself.
The bad reality
Usually, pre-commit hooks are a terrible idea. To clarify why, here are some my thoughts on this topic
Most likely your tests suite doesn't run in seconds
Tests can be speed up (see my other articles) but usually if you are working on non-trivial code base, the tests will take a considerable amount of time. Maybe it's 5 minutes, maybe 10, who knows, but you really do not want to do it before commit.
You already have CICD that will run exactly the same suite, why bother running it locally? Use the resources you yourself set up.
Developers will be either afraid of commiting often or will just skip this stupidity with --no-verify
This is a very important point, maybe even the most. You want your developers to commit often. This allows more experimentation, which can be reverted if they want, at any time. With long hook any commit is a long investment of time, nothing can be done in this time as well.
It will hinder your developers performance because of that. They will make less commits, make big commits, and will not be able to tests various solutions to a problem with easy way to revert them.
Are you sure tests can be run locally, or you only pretend?
Sure there can be projects that are set up correctly that everything can be run locally.
But even more often, nowadays, you will encounter absolutely rubbish setups that require, for example, AWS credentials to run tests.
Not only this makes it impossible to work locally, for example in a plane or on a train without internet, but also means that developers that are joining the project will need to wait for accesses to remote systems before even trying to commit anything! Hilarious situation, and complete waste of time.
Most likely one developer will not completely overhaul the whole system, in a way that every single test must be run locally when developing that refactor. Most of the time developer will work on one feature, quickly iterate with tests just for this feature, and once it's ready, will commit and push. Only then on CICD the whole suite will run, and your pipelines are already well configured for this task.
I should probably write a checklist style article about how to set up a project correctly. Follow me, it will happen soon.
Scope of testing in very big projects
Sometimes you encounter a monorepo with 10 subprojects inside it. Why the heck would a developer even want to run tests of completely unrelated projects when developing a small change in one of them? Again, waste of time. Anyway, CICD will validate stuff in the end.
Not to mention such monorepos are a stupid idea too, why group unrelated projects inside one single repository? There is no good answer to that.
Trust issues
Do you really want to show that you do not trust your developers to do the right thing? You should want them to experiment, push half working stuff to their branches, see the outcomes, who cares whats going on in their experimental branches.
The end goal is the same, working code, with passed pipeline tests, merged into main/dev/whatever.
By micromanaging your developers by enforcing pre-commit hooks you show that you don't trust them. What's the next step? Watching them code via screen sharing? Believe or not, I've seen that.
Final words
Just don't. If you already made a mistake and set up this feature already, remove it. Everyone already does --no-verify anyway.
Top comments (4)
I'm sorry but hard disagree with you. For me, it basically looks like "this tool can be overused or used incorrectly so the tool sucks". You can say something like that about almost anything... Furthermore you make a lot of arguments "don't you trust people" when it has 0 things to do with trust, the core system is about providing early feedback and to speed up developers...
But let's move to some arguments you make:
Re-1. Saying point is stupid if you don't fully understand it is... bad. You don't necessarily focus here on the git history of your
masterbranch. Consider a case, where you have built-in auto-formatting to save developers time if they forget to format something incorrectly (or have incorrectly setup IDE/use some custom one). If you do a quick "is code formatted/reformat my changes to the ONE TRUE FORMAT STYLE™" then if you do it on the server side, and then some CI test fails, you'll have to do extra steps like pull changes to your local branch before doing anything or force push remote branch or any other weird stuff because branches diverged. If you do this check/reformatting locally, everything is correct and developer can actually work on a followup code changes without having to deal with external changes to his/her code. Same thing with any linters that can automatically apply code changes.Everything depends on the situation, right? If you have 50 devs then probably not much, but with bigger scale...
This is the biggest lie of this article. Properly set up pre-commit hooks can actually save developers TONS of time. I can't count the times I pushed some changes, switched branches and started working on something else, only to get some notification that formatting, linters or some files were not regenerated in that commit, having to stash/save my just started work in progress, switch to my previous branch, fix things, submit again, wait for all tests to run again and then merge/deploy them. This is a HUGE waste of time. It's best to give developer feedback about changes ASAP. Ideally in IDE, but not everything can be easily supported in any IDE (and people like different IDEs, etc.). So if you can't do it in IDE, but you can quickly give some feedback or ensure some automatic tasks are always done before commiting, then why not? Not to mention all the context switching which is a huge waste of time.
Shouldn't they? Especially do quick/instant formatting/linting checks? And to be clear - I don't mean slow and long in-depth tests, but running even unit-tests of targets that have to be rebuilt (within some distance in dependency graph) is just good practice and saves everyone a lot of time.
It's not even about trust. People often forget simple things when they focus on more complex things. Adding automation so that developer doesn't have to remember about something he/she should do anyway is bad now? That makes no sense.
Who said you should run your whole test suite in pre-commit hook? It should be almost instantaneous to run it. If it takes more than 15-20s then it's bad. But you can do a lot in 15s. And these 15s will save a lot more time in long run.
For faster feedback, so that dev doesn't have to switch contexts. In bigger scale, you don't get your CI results right away, even scheduling and spinning up "machines" to do simpler tests can take some time.
If you don't commit often because your precommit hook takes 30s then... I have no words. Go make yourself a coffee/tea/stretch your legs/let your eyes rest a bit if that bothers you so much.
Also - taking about trust in developers when you assume they will blatantly ignore good practices and established rules/workflows...
Again, how is that relevant to whether pre-commit hooks can be good or not? You shouldn't run integration tests in pre-commit hook! Maybe unit-tests if they are super fast and/or you can do selective testing of only targets that are close to changed code.
Again, why is this at all related to pre-commit hooks? Same as whether monorepos make sense or not? Completely irrelevant because you shouldn't run EVERYTHING in them anyway, only a very small and fast subset.
I guess you're smarter then a lot of huge companies that actually do use monorepos... Of course there are pros of monorepos. Same as cons. Smart person considers both sides and chooses the best solution in given context, and doesn't always do X because Y is definitely worse so there is no point in thinking about that...
Wait, what? What?! This makes absolutely zero sense. Why wouldn't you give developer a quick feedback (albeit limited to what can be done very quickly) early? So I guess you're in favor of disabling all the helping tools in your IDEs like on-the-fly correctness checks/whether it will compile or not, automatic reformatting on save, etc. because it takes away dev's freedom? This makes zero sense.
You're also keep forgetting, that usually someone has to read those PRs too. Not sure about you, but I'd be pissed if I had to review someone changes that are formatted in awful and hard to read way because someone forgot to reformat everything before commiting?
Why have CI at all? Don't you trust developers that they sufficiently tested changes and ensured they are 100% correct?
I have to disagree with you on this. No professional developer would use —no-verify, if your commit has a problem then fix it, only proper code be merged into main workflow branches.
And about the CI/CD part already running the suit? It not even close, pre commit hook is made so that developer can detect their own issue locally before they decide to merge it if they made all the PR stuff on the git service just to discover that their code can not be merged after CI/CD fail, which should be faster?
You seem to be confusing commits and pushes with merges to the main branches. With properly set up repository you will not be able to merge a pull request until CICD passes, so what's the point in running it locally? It is also not much faster, because the pipeline will run anyway, so you end up running it 2 times, locally and on CICD. You will not end up with bad code merged into main branch if you disable precommit hooks.
What's the point of syntax highliting in your IDE if your CI will verify if your code compiles and works?
The point is to get as much quality feedback as early as possible. If you know right away that your CI tests (or review) will fail due to wrong formatting, not updating some generated files, using forbidden constructs/features, etc. why not provide it as early as possible? Is it better to let the dev switch to something else and have them come back just to fix something small that could have been easily detected earlier? Wouldn't that be super annoying waste of time in the long run?
Everything is a matter of balance. Pre-commit hook must be fast not to be annoying, but there are lots of things you can do that are fast and yet provide valuable feedback or automate some tasks so that people don't have to remember.