Precommit Hooks Are Bad

preview_player
Показать описание
Precommit hooks are the actual worst thing and I hope this convinces y'all to use them less.

Рекомендации по теме
Комментарии
Автор

This comment section has quickly taught me that y'all don't actually know how git works so I might have to make a video about that next

tdotgg
Автор

I don’t really see why you can’t have the checks both in pre-commit and in the CI. Personally, I prefer knowing that I have a failing check straight away, without having to wait for x minutes until the CI pipeline fails. And if your workflow involves “trying things and making lots of commits” you can very easily disable pre commit hooks on your local env and only rely on the CI pipeline.

TheMrSkyArt
Автор

This guy is the personification of Reddit comments whenever they see a problem.

Not only does he have to make sweeping generalizations about the topic, he also has to insult the people using it.

ccash
Автор

I would rather not have to comeback to a PR that I just pushed because I made some minor linting mistakes/ forgot about an unused import somewhere. imo it often saves time, and you can easily opt out and skip it with --no-verify.

alexoscreed
Автор

I prefer pre commit hook in addition to CI step for linting.

1. Cleaner commit history
2. I get feedback from my precommit more quickly than I get feedback from CI, even if it’s splitting hairs it’s still more convenient
3. CI picks it up if you try to bypass the hook

John-xxlw
Автор

1) Auto-formatting your code on CI seems like a bad idea. You need a whole additional logic of creating a new commit with some message on the server, that would contain the new formatting changes, and then pushing them to the right place. The author of that commit would be some bot. So the history would be mixed with all those bot formatting commits. Which is not that bad if squashing, but what's really bad is you might not be aware of these new commits, continue developing after the push, and then be faced with some merge conflicts and/or the need to rebase.

On the other hand - with pre-commit hooks you will just commit properly formatted code from the start automatically, without this unneeded complexity.

2) So the main downside that's brought up is when you are trying to commit WIP code and linter doesn't like it? Just use the --no-verify flag when commiting, and you are good to go. The CI would lint the code additionally later. Linter on hooks just saves you some time - you don't have to sit and watch the CI pipeline to check if all is ok (lint-staged checks only changed files, which is obviously much faster). And the situation when you need to commit code that ESLint errors on (warnings are allowed) is not that often IMO, but yeah i'll just use the --no-verify without getting rid of the benefits.

Kayander
Автор

git? i just copy the folder every time i change something. no problems :)

sircalvin
Автор

For me it definitely makes sense for detecting hardcoded secrets in your code. Sure, you can detect this stuff on a runner in CI, but by then the secret is already stored in git history and you have to go back and purge all reference to the secrets in git history. Developers make mistakes. Also, I prefer to do formatting and linting early and often. Made even easier by tools like Prettier and devcontainers for consistent environments.

mistymu
Автор

I agree with using CI for tests and linting. HOWEVER... Using precommit hooks to do stuff like detect-secrets and things like that, that matter BEFORE you push is still important.

This is ESPECIALLY true for engineers that are newer to the industry, don't fully understand whats going on and whats important, think of some precommit hooks as bike stabalisers over mandatory work flows.

MattBidewell
Автор

Based on the comments, this seems to be a completely normal workflow in many projects:
1. Most developers use text editors with no support for linting and autoformatting
2. The few non-nano users arbitrarily apply their own editor's formatting rules to the code
3. There's no linter config... except the magic one that may only be used by the pre-commit hook and the CI job
4. The CI job spins up 15 virtual machines to run the 30 minute build job, then fails the lint rules at the end out of spite

SWGINSPECTOR
Автор

I think there's a usecase, personally I like to use pre-commit hooks to enforce formatting. If it's going to be in git history, it should be formatted the same way as other commits in the code base. If you were to use CI for that kind of stuff, then yeah, your final commit will be formatted same way as the rest of the codebase, however the git history before CI ran will not.

tomasdiblik
Автор

Totally disagree, sry. 1) (Like someone said below) Add ticket number to commit. 2) Force conventional commit prefixes (feat, fix, chore). 3) Eslint 4) TSC. You say that you slow down developers, lol? A hook usually takes seconds/up to a minute. Now if you run those in CI instead, you probably won't sit there and look at it, so you might do other work, or go for lunch, or whatever, and you only notice it HOURS later. Now you've delayed the whole PR potentially. Oops its 6pm, lets do it tomorrow. Now you've wasted a whole business day that the company has to pay for. Not to mention the cost of the CI minutes themselves.

Frexuz
Автор

Commitizen is a project you'd hate, then.

This is more a question on habits and priorities. Not efficiency. Because if people followed the conventions set in place, they won't be stopped in their tracks. It's only if they execute outside of conventions that people are stopped.

CI is expensive compared to a git hook. If you care about the environment, you don't run 20 CIs per commit, because some person doesn't understand the conventions and pushes wrongly 20 times in a row. It's so much lost computing compared to just checking the commit (this is about commit message conventions obviously, not code style or whatever, at which point I agree).

neonraytracer
Автор

Every tool has it's place and purpose - I don't think you should be that harsh. I aggree that pre-commit hooks shall not be used to do any long running checking or CI type of workflow work, but using a pre-commit hook to achieve some simple mundain task that doesn't slow down or influence your commiting practices or flow is perfectly fine. For example we use pre-commit hooks to prepend ticket number in the commit message and that just mitigates a need to remember the number of the ticket that you're working on each time you make a commit :)

vytautas
Автор

I see some of your arguments since we use husky for some low hanging running lint tasks. We only run lint on staged files since we have so many files. We ended up adding husky because the CI bill was becoming too large to start ignoring small lint errors

WebDevCody
Автор

CI fails after 5 minutes. Now using husky makes sure that no build time error due to linting happens after the commit is done.

codedusting
Автор

answer me this: if you have lint rules that you ENFORCE, and are not going to pass CI anyway, why not enforce them BEFORE the developer commits? this both saves both CI build time (money), and the developer's sanity (not having to push a fix lint commit).

sercantor
Автор

am I dense or I didn't hear why the hooks are bad? it felt like you didn't justify the opinion

grmancool
Автор

Prettier pre-commit hook takes under a second to run. I'll keep running it, thanks.

Quenjii
Автор

Do you have the same opinion of pre-push hooks? I think formatting is a good use case for either precommit or pre-push hooks. Cause I don't want to push my code just to have CI tell me that I have formatting errors 10 minutes later. I have other things to do than just wait for my job pipelines to finish.

Edit: Reflecting, I think that the use cases of pre-commit hooks is where it enriches your commits vs blocking them.

goldydog