The dangers of precommit hooks

32,097
88
Published 2024-07-17

All Comments (21)
  • @leosin5767
    Unpopular opinion: you can still have pre-commit hooks for those who want a faster feedback loop (you can still --no-verify if you really wish to) while having checks to gatekeep on CI. It's never a mutually exclusive thing.
  • @wealthassistant
    I added a lint check to my local pre-push hook because I got too embarrassed for CI to be failing for that reason.
  • @forivall
    Prepush hooks are much better than precommit hooks (30 seconds at most)
  • @MadEye27
    it works till you have that one guy in the team that always force push because he’s in a hurry
  • So many people with editors "set up correctly" importing random stuff at top of the file cause of auto import completion 😭, I'm ok with automatic editor stuff, but please read the diffs before pushing...
  • @chekote
    If “good devs” respond to something they don’t like by quitting instead of participating in continual improvement, I would argue they are not good devs at all. If I worked with a dev like that, I’d be happy they quit. They sound like privileged toxic people. No thanks.
  • @manoellopes211
    I really don't understand the hate for standardizing commit messages and code linting rules. If you work in a medium-sized company with at least 4 or 5 people committing to the same repository, someone will review your PRs and that person is not obligated or has all the time in the world to try to understand what you did or stay notifying you and joining a video call so you can explain what you did. Good documentation is part of any mature software and yes, commits are also part of these documentation.
  • @3ventic
    10:09 the amount of times I've had to manually use "save without formatting" when working on some file with 10k lines and legacy formatting that doesn't match the current standard of the codebase lest I have to wait a good few seconds before it even saves and bury the actual diff in code review.
  • @cyrus01337
    I'm pretty forgetful thanks to my ADHD, and tend to forget to run my format command to ensure all of my files are consistently styled, so pre-commit hooks have worked amazingly for me in the same light auto-formatting with Prettier, or how ESLint finds problems in my code. Not having to think about it and letting it be done automatically is good, letting it take several seconds may also be fine, though by the time I'm done committing I'm onto the next problem/file (unless I borked something which it's then my safety net). In a corporate situation I wouldn't know, though personally I think this take is extreme for a mild convenience.
  • @dbarjs
    Why nobody is talking about lint-staged + pre-commit + eslint? It costs some milliseconds of the dev time and prevent to commit code with ESLint errors. I use this on my projects and I forgot this pre-commit hook exists some times, because the VS Code has the correct config (based on repository configs for devcontainer + eslint + vscode). Probably, the hook will only trigger something when the editor is not configured correctly (or has some extension not working properly at time). Sure, the CI will run fast after the push (1 min) and will stop the PR if some ESLint error exists.
  • @aquual1462
    I don't like format on save. But I'm addicted to the format shortcut.
  • Theo, so you have a branch where you changed a markdown file. Where would you check the syntax of the markdown ? 1. on the IDE (using some extension), but theres a risk, devs will not "care"/notice even thought the IDE is showing it. 2. in a githook that uses linter via cli that will prevent you from even creating the PR 2. in the PR build on the branch (slowing dev slightly as it is checked and causes a failure) 3. in the PR to merge from the my branch into main (will run there by default as last chance to detect), but will require dev to go back to this issue and fix it and break their existing flow not all developers are software engineers. not all languages are JS or have tooling to break detecting it closer to source is actually helping in reducing pipeline time and code review as developers get more experienced, the amount of needed work by githook will be reduced but mistakes can happen (but will just be detected slightly later). Its the same with say SonarLint. Adding it in a githook allowing you to get specific feedback as you code but also as just before a PR means less issues to be reviewed or potentially fail at the CI level
  • I recently turned off auto format on save. Why? They added new format rules. They didn't format the existing files. They added new code checking and coverage rules. I can pick up an old file, change one word in some copy and suddenly I'm responsible for 300 line changes which have triggered code coverage rules for ancient stuff written 5 years ago. Not to mention my pr is now polluted with formatting only changes and is unreadable. My 5 minute change just became a multiple day issue to resolve. Screw that, I'll just not format on save anymore.
  • @AlistairLynn
    I like pre-commit hooks in my own working copy sometimes, if I’ve forgotten things or want them done automatically. I feel like the difference is in the autonomy. You must run this hook which is difficult to change is a very different proposition to another tool to control your own workflow as a dev.
  • @KevinVandyTech
    So are we going to get a new mustache version of the why I hate unit tests video?
  • @dealloc
    $ git commit -m "quick fix" - tsc --noEmit | prettier --write / eslint --fix - upload-cat-images.sh \ sell-and-buy-nfts.sh | mine-cryptocurrency.sh error: failed to buy NFTs: not enough ETH.
  • @BR-lx7py
    How do you do safety net for minimizing the effects of a secret being committed to git? Scrubbing git history is a huge pain.
  • @drwalrus5943
    At work in the bank I work at, we have pre-receive hooks set up in bitbucket that require a jira ticket number at the start of each commit. We also have other things like no force push on any branches (even our own) all in the hopes that if the bank has a production incident it is supposedly auditable. I don't think this is a good reason, it's just a reason. In my opinion it inhibits developer freedom to improve the program without a specific ticket. Makes refactoring a nightmare too
  • @snatvb
    I use precommit hook for auto formatting when you will commit files it will be formatted automatically. I like it, just I have a lot of devs and I don't want to have zoo formatting in my project but CI should check if everything is fine. Precommit just gives pissibility to get rid of unneccessary manual work