The dangers of precommit hooks

[@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.

[@joaopslins]

Agree. That's how we work in my company. Honestly I'd rather waste 15 seconds locally than potentially waiting 30 minutes in CI to realize the build fail for a small detail. If someone else does not want to do that, that's fine, CI will catch them later.

[@seeibe]

​@@joaopslins If the CI takes significantly longer to performing the most important checks like linting and build, something is wrong with the CI setup. Lint and build normally should go into parallel stages right at the start to get feedback as fast as possible.

[@joaopslins]

@@seeibe agreed, they should be as early as possible, my statement was a bit exaggerated. That said, I still think the point is valid.

[@LLlAMnYP]

@@seeibe it's not because ci takes THAT long. But it still takes no less than a minute or two to start the CI job and that's enough time to context-switch away to something else and forget that a linter has ran.

[@seeibe]

@@LLlAMnYP Again seems like an issue with CI setup. We run a self hosted gitlab instance and self hosted runners, jobs start within seconds of the push.

[@lemontec]

I added a lint check to my local pre-push hook because I got too embarrassed for CI to be failing for that reason.

[@Lucas-gt8en]

dude same

[@tvujtatata]

I am just doing that manually so that was the only reason I could think of lol.

[@tvujtatata]

But also sometimes I push unfinished code to save work remotely and perhaps code that can be pre-reviewed before it is actually completed.

[@jonathansmith4832]

While I would prefer a properly configured IDE and editor that prevented such an error, it is reasonable that a developer who had this problem would choose to add this step personally to their own machine. A pre-push hook is also far more reasonable than a pre-commit hook for this. You also recognize the importance of this being in CI. This is a reasonable take, and devs on a team should be able to choose how they themselves work when the decision only has implications on them. It would be interesting to figure why the formatting and linting issues aren't always caught or don't surface until the pre-push hook.

[@meri5012]

...Why is your editor not doing that tho?

[@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.

[@TimeLemur6]

I get the feeling that a lot of the environments implementing this sort of stuff aren't the type that are interested in improvement.

[@chekote]

@@TimeLemur6 if that’s the case, then yeah, that’s not a good place to work.

[@d3fau1thmph]

they are good 100% on a tantrum spectrum

[@quinten01]

This 100%.

[@elirane85]

Agreed. I also hate pre-commit hooks, so instead of quitting or even mention it to anyone, I just quietly configured my machine to ignore the hooks. And people like me are exactly why using pre-commit hooks and not a proper CI is a dumb dumb idea 😜

[@zaneearldufour]

pre-commit hooks are faster feedback than CI

[@jordanbrennan1296]

Yup. Why wait for CI to discover something simple is broken when a pre-commit hook could have told you? Write commit message > push changes > 30 mins later realize the PR is red > read GH action logs > discover something small and stupid you missed > go back to that file and fix > start a whole commit. That’s what happens without some basic hooks.

[@Mystic998]

Yes, and you have to rely on developers doing the right thing for them to work properly. So you do both. Never trust a user to do the right thing.

[@ChrisPepper1989]

i prefer pre-push in this instance. because I want to encourage my devs to make lots of little commits, even if they are not ready for PR, not discourage them :)

[@ninocraft1]

im able to run the same scripts the CI does on my machine so i can get faster feedback

[@FunctionGermany]

what if - and this will sound crazy - you do *both*?

[@forivall]

Prepush hooks are much better than precommit hooks (30 seconds at most)

[@elirane85]

And both of them can be ignored with a flag. You need something that doesn't run locally so developers can't skip it.

[@forivall]

@@elirane85 oh absolutely. The pre-push is just so that devs don't need to wait for ci to finish to see what they did wrong. That's why I say 30 seconds at most.

[@elirane85]

​@@forivall I definitely don't share Theo's vitriolic anger towards hooks. But I do 100% agree with him that CI first, then, maybe, if you want, add hooks. And I mostly don't care about the hooks since I almost 100% of time skip them anyway ;)

[@rak3shpai]

This is like saying we shouldn't do client-side validation because it's no substitute for server-side validation. Sure, the latter is true - the checks have to run in CI, anyone debating that is wrong. But surely it's a better devex if at least the cheapest, fastest running checks can run locally as well so that the dev can get feedback asap. Surely it's not a better experience to push, CI's going to take time so you move on to other things, then discover via email or some other notification that CI failed, stash/commit and get back to the PR branch fix issues and push again, rinse and repeat. I'm in agreement that pre-commit hooks shouldn't come in the way of developer flow. The time it takes should be of the order of low single-digit seconds at worst. But that's not an argument against pre-commit hooks, just against slow ones.

[@dariofagotto4047]

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...

[@d3stinYwOw]

auto import is the devil.

[@meri5012]

> please read the diffs before pushing This is true whether or not you're using IDE automations That or at least read your own PR diffs

[@seeibe]

Oh yeah, remind me to turn that off.. 100% of the time when I auto-complete some text that requires an import, I want to select the import manually from a drop down. My muscle memory has become ctrl-c ctrl-z ctrl-z ctrl-v just to undo the dumb auto import from the completion.

[@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.

[@d3stinYwOw]

As well as making it just a nick harder to commit without passing linter and formatting check, because, as good as editors are, people forget to format according to accepted way in repo. I can see it at work sometimes, where CI picked up that formatting was off, or lint, and next few commits were made just to fix those, already in PR.

[@shaunkeys7887]

Having specific rules for commit messages means every change has to be classified and easily explained in such a way that it fits into those rules. That means I have to spend that much time coming up with some clever way of describing my change so it can be classified into one of the categories, which takes more time. You know what makes that process easier? Grouping all my changes into one commit. Never make the worse decision easier for devs or it will come back to bite you later

[@manoellopes211]

@@shaunkeys7887 I really don't see how you could spend that much time writing a good commit message. It's just a case of you and your team choosing a pattern like Conventional Commits or Git Flow and using it. Your commit messages don't have to be perfect, but it makes it a lot easier to understand what you've done and review it if I know right away whether your commit is to add a new feature, fix a bug, or just refactor.

[@manoellopes211]

​@@shaunkeys7887 I really don't see how you could spend that much time writing a good commit message. It's just a case of you and your team choosing a pattern like Conventional Commits or Git Flow and using it. Your commit messages don't have to be perfect, but it makes it a lot easier to understand what you've done and review it if I know right away whether your commit is to add a new feature, fix a bug, or just refactor

[@manoellopes211]

@@shaunkeys7887 I really don't see how you could spend that much time writing a good commit message. It's just a case of you and your team choosing a pattern like Conventional Commits or Git Flow and using it. Your commit messages don't have to be perfect, but it makes it a lot easier to understand what you've done and review it if I know right away whether your commit is to add a new feature, fix a bug, or just refactor

[@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.

[@Blender3DProjects]

Exactly the same issue here, also it triggers code coverage rules on my repo so now I have to get 80 percent coverage on legacy code because I changed one word in some copy. Turned that shit off and my life is so much easier

[@krissh_the_dev]

I relate to that. I use VSCode, and I have a separate a work profile in which format on save is disabled.

[@d3fau1thmph]

diff with ignore whitespace is too diff-icult for snowflakes also cleanup branch is apparently too alien of a concept :shrug:

[@3ventic]

​@@d3fau1thmph Code quality checks in CI will often pick whitespace changes and mark the function as changed and subject to the quality checks as a result. There's often no time or budget for cleaning up decades old code, and in order to even get the permission to do it you'd sometimes have to convince some team on the other side of the world. There can be a lot of bureaucracy involved in any change so you end up minimizing your impact to unrelated code as much as possible. You could just say that you don't want to work in that kind of environment, and I'd agree, but it's the reality in many large companies employing tens of thousands of people each.

[@d3fau1thmph]

@@3ventic Yeah, no time or budget to remove technical debt. Good strategy to dig yourself deeper. You talk like a marketing/sales manager.

[@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.

[@td19xyz]

Theo might not be the best at saying this, but he's not saying to not use git precommit hooks, but he's saying that it shouldn't be used instead of CI

[@MadEye27]

it works till you have that one guy in the team that always force push because he’s in a hurry

[@adivmt]

😂😂😂

[@bruwyvn]

yep

[@bruwyvn]

Still needs to be careful because that can lead to a slippery-slope

[@ideiasBrasil2023]

You can block it by the way if you are a repo admin.

[@bruwyvn]

@@ideiasBrasil2023 not if it is private repo

[@Paul-qj4dr]

Another issue with pre-commit: "works on my machine"

[@michawhite7613]

Good tooling shouldn't have this problem

[@emonadeo]

@@michawhite7613 „Good tooling“ 😂😭💀

[@Paul-qj4dr]

@@michawhite7613 it's not just about tooling. There are so many things which can break code.

[@zyansheep]

If you use NixOS dev envs, this _should_ be less of a problem...

[@mistertoups]

@@michawhite7613 you ever try to write a shell script that has to work consistently on a variety of unix environments? I know there are ways to make that work but they all suck and do you really want to go to all that trouble to add a workflow step that makes everyone's day-to-day coding experience excruciatingly miserable?

[@Blender3DProjects]

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.

[@DefnDKMC]

That's not an argument against format on save, that's an argument against idiotic coverage policies that prevent you from formatting old files. When the formatting rules change they should should immediately be applied to the entire repo in a separate commit anyways.

[@SpomenkoJabucar]

Skill issue

[@Kitulous]

@@DefnDKMC which then probably results in a heck ton of merge conflicts. which can be solved with reformatting but anyways.

[@Blender3DProjects]

@@DefnDKMC Yeah well the new formatting rules actually broke code due to things like import order changing on a lot of the oldest stuff. Manual intervention was needed to resolve the myriad of build issues across thousands of legacy files and so going through and formatting them was put on the back burner. It's a half baked plan in the name of quality code that has resulted in a terrible dev experience.

[@b33thr33kay]

Man, take a chill pill. I already have enough anxiety.

[@b33thr33kay]

I hope you realize that the vast majority of your audience is made of inexperienced devs who have no right to be so adamant about their opinions.

[@randomgamer518]

​@@b33thr33kaywhat weird take, how is this going to give anyone a anxiety? Just take in the opinion, think about, come to your own conclusion. If listening to opinions like this is somehow anxiety inducing or offensive to you, you have bigger issues and it's not anybody's responsibility to cater to that

[@DeciPaliz]

​@@randomgamer518 i mean, theo was quite aggressive about pre-commit hooks over ci being trash, talked about firing people. i wouldn't say he's overexaggerating, i'm not at all a competent programmer, but i work in a company that uses pre-commit hooks without ci, and yeah, uhh, that's quite ugly. but people who didn't have enough experience with this and thought that "running stuff locally opposed to running stuff remotely is always better" might feel a bit overwhelmed by the way that theo was opposing this idea

[@elirane85]

@@DeciPaliz pre-commit hooks are completely pointless. A good developer will check the code before committing anyway, and a bad developer will just ignore them and force push the code. So all they do is slow down the development process, and provide a false sense of security if you just assume that the committed code is good without running it through CI. I've been a developer for almost 20 years, half of it doing DevOps, and I personally have all the "pre" hooks set to ignore by default on my computer.

[@DeciPaliz]

@@elirane85 i'm not arguing with that, i just think that the statement that theo made was delivered a bit too emotionally, which made the original comment's author feel anxious and perhaps discouraged

[@maciejcisowski7015]

Yeah, hooks instead of CI is a dumb take, but so is the "I hand my notice if you have pre-commit hooks" one. Depending on the stack used, pre-commit hooks can be a blessing. To give an example: I have a Python project with pre-commit checks for linting, formatting and type safety (mypy). Can I just have them as a CI step pre-merge? Sure, but here are some problems with that: 1. People can still work around CI checks failing and just force merge (and you want that ability sometimes). So it's the same argument as with people using --no-verify. 2. If you have larger, feature-level PRs that include multiuple commits, having mypy throw hundreds of errors at the CI level is a pain. Pre-commit hooks address that: we fix the issues at the commit level and don't aggregate them until we hit the CI wall. The scenario JLarky mentioned means they don't pin versions of their dependencies and that's the root issue there. And if someone is just too good (in their opinion) to install and run pre-commits when working with me on a repo than yeah, I'd rather they give their notice than have to clean up after them or react to every PR review request with "fix linting / formatting / types". At the end of the day these are just tools. It's more important to have a consensus around their usage in a team and follow that consistently.

[@arieheinrich3457]

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

[@Mystic998]

As he said, he doesn't really care if you use pre-commit hooks, though he generally doesn't like them. They're just not a replacement for CI. Your CI should check if a PR is at least basically ready to be merged, and that includes formatting, linting, coverage (ick), or whatever. You can't JUST do those types of things in hooks that are easily bypassed. I also get the distaste for things that stand in the way of you simply saving your work remotely. I think the ideal workflow would probably be to include pre-commit/pre-push hooks that provide warnings for things like linting and formatting and error out for security issues like trying to commit api keys or whatever then cover all those things in CI as well.

[@aquual1462]

I don't like format on save. But I'm addicted to the format shortcut.

[@palyanytsia]

shame on you

[@pedro.britto]

Me too. I don't like things jumping around when I save my files. But I do expect that to happen when I hit the format shortcut. And thank goodness for the "Format without save" shortcut.

[@fredrik6936]

Agree with most use cases, like running tests or linting. We do however use pre-commit hooks for naming commits with conventional commits. This is great because it allows us to auto version our applications and allow CI to decide whether or not to create a new gh release and deploy the changes. It also allows us to automatically create a changelog with each release based on the commit messages. Once deployed to prod, the release version is pushed to Sentry and defects are marked with the version, improving observability. Versioning manually is a pain.

[@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.

[@AndreyMikhaylovlolmaus]

THANK YOU! Why does Theo not realize that a pre-commit hook and CI checks are not mutually exclusive?!

[@remrevo3944]

10:00 Prettier on-save times are *slow*. Still wouldn't want to live without it. I don't want to have to care about formatting.

[@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

[@diego-aquino]

Exactly!

[@KevinVandyTech]

So are we going to get a new mustache version of the why I hate unit tests video?

[@astral6749]

I added pre-commit hooks on our greenfield project because we don't have CI. My seniors already considered adding it and everyone already gave their okays, but for some reason, it's still not there. Our other projects are already going to hell because of it, and I'm only an intern so I don't have permissions to add it myself. Pre-commit hooks is better than having nothing.

[@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

[@elmalleable]

i have been tthinking about this kind of problem and thinking what is there is a talk with the leads to establish an alternate process to deal with refactoring and other workflows that do not fit in this kind of ticket based commit and what not right? this should allow the auditable commit stuff and devs will still be able to do their refactoring and things but the refactoring goes through a different route which might ultimately round of with a issue number based commit on final review and approval

[@martinzihlmann822]

so you keep all branches locally until you're happy with the history. there goes your audit trail...

[@tatsu13]

This is required in my team as well. It’s… not perfect but I actually added a pre commit hook that adds the ticket number so long as the developer named their branch off of the ticket number. That also means we don’t allow devs to push branches that don’t have the ticket formatting at the beginning of the branch but it reduces toil at least.

[@elmalleable]

@@tatsu13 i cant hate this, a win is a win

[@seeibe]

Any software product development team should have a refactoring budget

[@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.

[@EwanMarshall]

Yeah, if you are using them in such a way just for yourself out of choice and it doesn't affect others, that is fine. But that should not be instead of proper CI testing if you can do it.

[@elirane85]

When you are solo developing, pretty much anything goes. But once you work as part of a team, you need to create an "idiot-proof" process that no one can skip/bypass.

[@AlistairLynn]

@@elirane85 It sounds like you don’t have a whole lot of time for your colleagues. I don’t think I’d call anyone I’ve worked with an idiot.

[@elirane85]

@@AlistairLynn You don't need to be an idiot to do idiotic things. I do them all the time. Maybe I am an idiot? ;)

[@wcrb15]

I'm glad you brought up the auth key check thing. It's the only valid use for pre-commits I can think of. I had a project where I had no control over who worked with us and had one dev that committed auth keys multiple times, even after putting the pre-commit hook in place. It was frustrating. I agree we shouldn't have to do that type of thing but it happens. For that project when anyone committed a key to any repo it set off crazy alarm bells and I would have to sit in multiple hours long meetings "fixing" the issue. They preferred that we revised the git history to remove the key from git AND to cycle the auth key. It was painful

[@wcrb15]

@@Spiker985Studios it was definitely the right way to handle it. Just a PITA because I suddenly had a bunch of high priority cleanup work due to someone else's mistake 😂

[@Synena]

I loved the analogy of frontend vs backend security

[@mitchelline]

I have a prepare-commit-msg hook that is opt-in (completely optional) that checks if the message exceeds 72 characters, makes sure you use a valid prefix, and performs cargo clippy && cargo test. I added this because I kept forgetting to test and format my code before pushing, and having CI fail several times over was just embarrassing, so I added it as a completely optional local step

[@kale_bhai]

--no-verify

[@Jplaysterraria]

I work with nix and Python often enough that I can't have format on save on as they both don't have standard formatters and I might accidentally turn a 2 line diff into a 200 line diff

[@James-ry3bo]

I counter catching new i18n keys for entry in a JSON file is useful as a pre-commit hook. Of course needed in CI too but useful to close the cycle for a small fix up.

[@Kenionatus]

A repo I sometimes work on uses pre commit hooks for decontlicting an unusual file format (a game map). It's quite useful, especially since it turns git conflict markers into something the mapping program can display when it can't fully deconflict.

[@jonathan-._.-]

we have an eslint check against an auto fixable eslint rule (and only one) so the pipe fails if the precommit hook didnt run with a smal guide on how to set it up correctly- cause sometimes new people forget to set it up (or didnt do it correctly)

[@TusharSelvakumar]

We have a pre commit hook to verify that the commit message is correctly formatted because if it’s not it would break our release notes.

[@statuschannel8572]

thats a good use case for a pre commit hook, i guess you guys using Conventional commit messaging format?

[@dj_256]

HAHAHAHA, was waiting to see your reaction on this one. I actually never set up pre commits hooks in a repository yet. But I use JetBrains IDEs that allow you to continuously reformat, remove unused imports, analyze... And to do that before commiting too. And I always use these features and encourage my teammates to do the same

[@MSheepdog]

I firmly believe devs working on anything big should have lots of commits in their local tree (often in broken states), and a remote branch they're pushing to periodically in case something happens to their machine. Pre-commit hooks add friction to this, and basically force you to skip the validation on like 90% of your commits. Pre-push hooks aren't as bad, but I still find very little value in them over waiting for CI to fail. Plus if you can run the tests locally, there's nothing stopping you from pushing, starting the tests in the background, and then writing up your PR description while your background tests finish.

[@Hapkumdo]

I couldn't agree more - this is your best video in the last month!

[@DerHerrGammler]

I only setup one hook in our corperate repo. A post pull hook, that looks for changes on the package.json file and makes an 'npm install' because I got anoyed by all the people forgot to run it after a pull. Never heard of anyone again that they had any issues with their version. Saves time for me to answer less stupid questions.

[@bruwyvn]

I would suggest for you to check against the lockfile instead of the package.json, but still a very good idea.

[@dubemdesign]

I use pre-commit hook to log out todos in my project as a reminder. Also to enforce some commit message standards.

[@tomiwaibrahim6198]

precommit hooks are really good for regression testing to make sure you know what commit made a UI change It doesn't block your commits it just creates new snapshots of your UI

[@jordanbrennan1296]

On file change is the most intrusive, distracting, time-wasting place to do it. It’s idiotic. Pre-commit hooks are totally fine, teams just need to determine if mandatory or opt-in is best for the project. Influencers say sensational and entertaining things in order to get eyeballs, so take it all with a grain of salt.

[@sajon_]

Just yesterday I had to wait for 10 minutes for a pre-commit hook to finish running before it let me push the code and open a PR. Such a nightmare! What is worse, the same checks happen on our CI! So I don't get the need to do the same checks twice and expecting different result.

[@ryanjean]

The only pre-commit hook I've ever been comfortable with (as in throw up hands and say "meh", not desiring it) is one to block merging onto protected branches. That is, of course, always backed up by similar rules in the CI solution, so pushes will fail even if they bypass the hook. And that, in turn, reinforces the unnecessary nature of the hook - as you correctly point out, if your CI is already covering that step, all that hook does is make the fail earlier (commit instead of push), and if your devs are having that many issues staying off protected branches you have much bigger problems - but sometimes that can still happen with entry-level devs who never learned git and/or really old devs who never learned git. For the other prominent example given: If you need hooks to avoid saving secrets, you almost certainly haven't configured your workspace right to start with.

[@luketurner314]

2:39 in his latest video, Brodie Robertson put it this way: "you are a guest on my computer"

[@ParthaSarathylink]

Looking at ridiculous complex devops process & never ending tools that glued together. this might area might worth exploring

[@LokiCDK]

First thing I do for secrets is implement a vault, and import from vault to my code base as a const. No need to scan for secrets because YOU SHOULD NEVER HAVE SECRETS IN CODE, EVEN IN DEV!

[@thisweekinreact]

As much as I hate precommit hooks, I feel like I still need them to avoid waiting for the CI to fail. If I commit and move on to another branch, and have to fix lint errors on the previous branch, that's additional context switching that I dislike. The main problem to me is that Github security model made it hard to wire lint autofixing on the CI. The CI shouldn't fail for Prettier issues IMHO but just fix the branch automatically. Regarding safety nets / guardrails: I know you dislike Storybook but I've found visual regression testing tools (like Chromatic) to actually be great safety nets. This catches problems you wouldn't otherwise.

[@garyduell3768]

Regarding switching between branches, aren't worktrees just an easier way to manage that? Start the CI, open a worktree for a different feature. After your CI, you can just close the previous worktree.

[@ailkoclaeys182]

My first coding job used a pre-commit hook that ran all tests etc, which took way too long and usually failed on something small, after that I commited everything with no-verify and never had issues (also we didn't have CI pipelines at the time)

[@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.

[@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.

[@somebody-anonymous]

If I run git clone and then commit something, will that run a script if a pre commit hook is set up in the repo? Very scary

[@dealloc]

@@somebody-anonymous git hooks has to be "manually" installed into the local .git directory. But that can be automated through npm pre-install scripts.

[@red_991]

I agree that pre-commit hooks do not replace CI but its good to have nonetheless. I like having the faster feeding greatly for not only lint/prettier errors but also enforcing the commit message to include the Jira url etc.

[@madumlao]

had precommit/prepush hooks mandated by my last job, which ran the entire test suite which would take 30 minutes. as far as i can tell nobody ever used the "mandated" pre-commit hooks

[@rajatsx]

Yesterday itself, I added "npm run build" to my pre commit hook.

[@nirjalpaudel]

In my blog site, I use precommit hooks to change png/jpg to webpages in fs and md files

[@KevinLyda]

Two notes: I use pre-commit hooks for spell checking my commit messages. The moustache. I'm 53m - a fair bit older than you or Prime. But at the same time you both look like adults from when I was a kid in the 70s. So it's weird hearing you talk about stuff more recent than Apple ][ Basic. Oh, and post-pull git hooks are great if you use vcsh for your home directory.

[@alexcarter8082]

Honestly love pre commit for early verification of lining related stuff. Especially for a wide spanning mono repo. Yes CI will catch and report but pre commit gives it a nice chance to just format and fix it before the failure. Auto format on save just sadly isn’t an option for all situations.

[@mappedmnd]

I still remember precommits silently failing and then getting blamed for it when it gets pushed

[@DmytroPalaniichuk]

When you have a team with 10+ dev working on same code base , even with setup properly shared lint/formating config per diff ide on save , lint stage is faster loop , as well save you compute time for building , when we have 20-50 builds per day with vercel . The good strategy would have pre commit hook on lint stage plus on save editor enables plus ci check as well . You even could check if no verify flag is added or not .

[@FlorianWendelborn]

14:30 I agree but in this exact example you can consider using a git worktree to work on your other branch already in a separate folder

[@Pekz00r]

The problem with fixing styling in CI is that you will end up with so many "fix style" commits in your version control. If you squash your PRs that is fine, but otherwise it looks pretty bad and adds a lot of noice. I'd still run it it CI as well to really make sure, but you really need to make sure you running it locally before commit as well. If you only run the linter on modified files it shouldn't take more than a second or so.

[@Synena]

I think I've got the point. I love pre-commit hooks and formatOnSave. I just realized they are meant for DX. Safetynets at CI should come first. Thx for the video

[@pencilcheck]

This is setup by tech lead or manager who doesn’t assume engineers aren’t staying long term, and will not be loyal that’s why they have this there

[@norude]

I have an interesting usecase for pre-commit hooks. I don't like the formatting rules, enforced by the git repo, so I use my own rules when editing and then format with their rules in a pre-commit hook

[@vsolyomi]

It took me this video to realize that in vscode I can save on focusChange and run format on save at the same time...

[@danieljanjanocha7178]

What about strategy below? - precommit: formatting (with lint-staged), - prepush: linting - in ci/cd: checking everything before deploying

[@emaayan]

we use pre-commit hooks to verify commit messages confirm to a specfiic pattern of Issue: issue number : description, always gives me anxiety before i commit but at least intellij has a plugin to verify this before i commit.

[@ymi_yugy3133]

Y'all quit so quickly. When I first started my job, we were using SVN with a pipeline to production. If I committed anything the change would run through CI and be live in production within minutes without any review. So my workflow was, do a change ping a senior dev, show and explain my code to them on a call then commit it. If none were available I would send them patches with my changes via slack. I did complain a lot and after a couple of month we switched to git and implemented a code review process.

[@rankala]

Well, I can see why some commit message formatting is important if it makes other jobs easier, like automatically have good written release notes. Or something like a linked issue number. I would not say, thats reasons for a 2 weeks notice.

[@some1and297]

I think pre-commit hooks could also be good for forcing line termination to be consistent with some simple find + replace. Same thing with tabs/spaces in python.

[@haleyk10198]

Pretty sure you can enforce pre commit hooks environments when your cooperate sells out hard enough for Microsoft, and those are usually the ones that needs a pre commit hooks to please your compliance teams even if it only covers 80% of the devs.

[@gsgregory2022]

I somehow didn't know about pre commit hooks before this. The only proper use of them I could see is if you wanted to replicate what your CI process does for checks before pushing, aka you the dev making a choice to do them. They aren't a process though. If you cannot enforce them and they can't be standardized across the team then they will break things.

[@mattilindstrom]

I'm not a software engineer, just a physicist. I was working with Python, so the question of does it even compile was irrelevant. The only relevant hooks we had were checking the commit message had the required fields, and test cases for every function. Everything could be pure gobbledygook. And of course I was absolutely butchered on every commit review, but that's what being a hobbyist in professionals' game is like.

[@darkshoxx]

At my last job using pre-commit was recommended but optional. But the repos were managable enough that running the hooks usually took less than 20 seconds. Either way everything was done in CI as well so no big issue. To me the main benefit to me personally was catching flake8 stuff that the autolinter couldn't deal with (arguably that's an issue with the linter 😅). Yeah 8 minutes would probably make me use --no-verify on every commit that isn't the last one before PR.

[@valseedian]

format on new line ftw

[@m12652]

When my code is finished it's pretty because I made it that way. When it's in progress I mess with indentation and vertical white space to make it easier for me to find the bits that still need attention (not if I'm in Python though 🙄). To have the editor reformat on save would ruin my day over and over again.... I worked in a team of devs for years when Prettier etc. didn't exist. We had proper code reviews, used generic code wherever possible and had strict naming conventions. The code was stunning and we could jump in on someone elses work like it was our own. It was fun... code reviews were mostly about finding occasional spelling mistakes etc. if someones code didn't look right we just rejected it. We did however spot the occasional bug waiting to happen. Having the editor make it look respectable just creeps me out... are devs that sloppy now? It'd be like dressing up a burned out old car in a new body... And the other great thing was if it looked like crap there was almost always something wrong with it.

[@RainbowPigeon15]

I would have loved to have that credential pre-commit hook, all teams (except the platform team) on my previous project constantly committed secrets, but it's fine because our repos are private 😑. I never thought that enough was done and I was mostly ignored every time I brought it up. Still waiting for them to get hacked, at this rate, its only matter of time...

[@tolstoievski4926]

I'm just using precommit hook to prepend the number of my ticket I'm currently working on to my commit message.

[@shgysk8zer0]

I turned off auto format on save because it formats incorrectly. There's probably settings for it somewhere, but it hasn't been worth my time to find and change them.

[@Renoistic]

I use a formatting tool and linters to do a quick check. I hate commit message checks with all my soul though. They are annoying and don't stop people from force pushing anyway.

[@3ventic]

There can be some value in having linting and formatting checks run in a precommit hook, but only as a callout that "hey, this might not pass CI" (no autofixing, no blocking the commit), because waiting on long CI pipelines only to find a minor formatting error sucks and sometimes the dev environment doesn't do it or you forget to do it yourself. 100% stuff needs to be in CI regardless though.

[@CouchProgrammer]

Hot take #1🔥 If you do pull requests, you don't have CI. Hot take #2🔥 If you can't run unit tests on-save and the required integration tests on-push, you need to start optimizing your tests rather than waiting for the full CI/DI pipeline. I've been in a situation several times where I forgot to do `npm i` after rebase. If you forgot to do this and committed your changes, you most likely made your commit unusable and will have to edit it. If you sent it to others and went home, you are not a good person. And the next day you find that your commit has been rolled back or fixed. Use lint-staged. This will save you a lot of time. This is also more beneficial on weak computers if the --watch is too greedy.

[@joegaffney8006]

I recently had to push back on a rule that forces us to include the Jira ticket name at the start of commit messages for feature branches

[@zenDevNomad]

That’s what we use pre commit hooks for! Is that bad?

[@joegaffney8006]

@@zenDevNomad it is a bit annoying if you have to type it every commit and can be handled by forcing this convention of the ticket name in the commit message of the feature branch into main.

[@sam02h]

pre-commit hooks are great for student projects where you can't trust other people to do the work they were assigned. I've had multiple team members treat "committing" as the end of their responsibility. Once it's pushed up, they believe it their work is done, having not tested it at-all. The trouble with CI, is the non instant feedback, it slow and frustrating for the less experienced people on the team. But you betchya it is there for the cheeky fucker who releases they can bypass it, or not even setup pre-commit. I've had cases where CI ends up as the target of people's blame, its a barrier by design and people struggle to see it's not the cause of their issue. pre-commit hooks, allows the less experienced team members to identify there is a problem, and ask for help if they can't solve it themselves. Even if there was a script to test, it wouldn't be used by the kind of people I'm targeting with pre-commit.

[@bruwyvn]

I had to add a pre-commit to block push to main in a private GitHub repo without a paid plan. Edge cases exist, still agree tho.

[@codedusting]

We had an issue in the client project where they weren't on any GitHub plan but default 3000 mins time monthly. We had more than 20 repos in that account and one of the frontend repo required 15-25 mins for build time every gcp deployment 😂 This meant restrictions on GitHub CI/CD timeline and nearly running out of it monthly in case multiple eslint checks in CI/CD pipeline. Eventually in that constraints, we had to add pre-commit hooks to check eslint linter and prettier formatter during push. Bad DX, waste of time, but in that client constraint of budget and whatnot, this had to be done. We didn't remove anything in the CI/CD pipeline just in case someone missed it because boy we used to comment the husky precommits during deadlines.

[@gwkunze]

pre-commit hooks are like browser-side validation, an extra but shouldn't be trusted. Dev environments can vary too much

[@attilazimler1614]

Try auto format on save on a legacy code that you are trying to fix a bug in. Be it in a situation where you have to do it regularly and you change your mind on setting up people’s editor on doing stuff on save other than saving really quickly.

[@tiborgrun6963]

Is my build script checking if the project is named with a specific suffix and failing otherwise a good solution to the problem of building from not checked-in project?

[@KarlOlofsson]

I don't even want linting. I want auto-formatting and correct code completition. It's such a mess when editor-autocomplete and linting "fight" and I have to manually change everything.

[@abdallahgamal7164]

You can enforce devs to use a specific dev environment using devenv

[@nivoset]

my only one is a pre commit message to add the jira number from the branch name to the commit message.

[@emeraldbonsai]

seems like most of the issue is people not know you can just automate the changes so it doesnt fail....

[@kibbewater]

I rely hard on my format on save lol. I write disgusting code and as little code as possible, press ctrl+s and have it all formatted and prettified. Honestly speeds up how fast I write code when I can stop thinking about making code pretty with newlines, brackets and other stuff that will just be automatically fixed by prettier when I save a second later

[@aj8__8]

our pre commit hook takes 10 seconds max, anything above 20 is unacceptable

[@MrDadidou]

Theo 2027 with a goatie: oh this is a stach vid. let's recheck my opinion on this

[@mschelstastic]

there's a repo i've been working on at work recently and it has pre-commit hooks for linting and testing, doesn't run them in the ci build, and when i started working on it there were all kinds of lint issues everywhere (some of them could not be automatically remediated) and i'm like ffs i don't want to bypass your team's clear intent (fully linted code) but clearly everyone else who works on this repo just bypasses them, so i end up with a tiny change that touches like 50 other files, grumble grumble.

[@enriqueflores4098]

pre-commit hooks can be used when you working on a small solo proyect and dont want to bother with ci/cd.

[@MichelleGenderbendM]

Even then, I'd recommend using CI so that you can switch immediately to playing games when you're done for the day, without your system being bogged down by a build system.

[@Lemon-lp1qb]

You won't believe it but your thumbnail is my today's shower thought 🚿

[@helleye311]

We have formatting pre-commit now, it takes about a second and I'm constantly annoyed by it. good thing is, I can still type while that's going on, usually it finishes by the time I write git push. I''ll still consider turning it off though, it doesn't add any value with auto formatting on save besides adding some rainbow text to the console.

[@chris.dillon]

I format on save and have as many tools early on but still no hooks. You aren't going to remember the CI steps. This is why I make a `./bin/ci` or a CI run target (whatever build tool) and then CI runs that. The idea is from a book Sustainable Web Development but I've applied it to other tech stacks. If you think CI is going to fail, just run the CI script.