zlacker

This might be a me problem but I extensively manipulate the git history all the time which makes me loathe git hooks. A commit should take milliseconds, not a minute.

>>schind+(OP)
You do seem to be doing it wrong. Extensive manipulation of the record and slow hooks are both undesirable.

>>esafak+0b
I would reckon cleaning up your branch before opening a pull request is good practice. I also rebase a lot, aswell as git reset, and I use wip commits.

Slow hooks are also not a problem in projects I manage as I don't use them.

>>schind+ff
No, I would not and don't do that. It is better to leave the PR commits separate and atomic so reviewers can digest them more easily. You just squash on merge.

> Slow hooks are also not a problem in projects I manage as I don't use them.

You bypass the slow hooks you mentioned? Why even have hooks then?

>>esafak+4g
I do leave PR commits separate. In my teams I don't set up pre-commit hooks altogether, unless others feel strongly otherwise. In projects where they are forced upon me I frequently --no-verify hooks if they are slow, as the linter runs on save and I run tests during development. CI failing unintentionally is usually not a problem for me.

>>schind+(OP)
it’s not just you.

i regularly edit history of PRs for a variety of reasons and avoid pre-commit when possible.

put it all in CI thank you please — gimme a big red X on my pipeline publicly telling me i’ve forgotten to do something considered important.

>>esafak+4g
> It is better to leave the PR commits separate and atomic so reviewers can digest them more easily.

So reviewers have to digest all of the twists and turns I took to get to the final result? Why oh why oh why?

Sure, if they've already seen some of it, then there should be an easy way for them to see the updates. (Either via separate commits or if you're fortunate enough to have a good review system, integrated interdiffs so you can choose what view to use.)

In a better world, it would be the code author's responsibility to construct a meaningful series of commits. Unless you do everything perfectly right the first time, that means updating commits or using fixup commits. This doesn't just benefit reviewers, it's also enormously valuable when something goes wrong and you can bisect it down to one small change rather than half a dozen not-even-compiling ones.

But then, you said "atomic", which suggests you're already trying to make clean commits. How do you do that without modifying past commits once you discover another piece that belongs with an earlier step?

> You just squash on merge.

I'd rather not. Or more specifically, optimal review granularity != optimal final granularity. Some things should be reviewed separately then squashed together (eg a refactoring + the change on top). Some things should stay separate (eg making a change to one scary area and then making it to another). And optimal authoring granularity can often be yet another thing.

But I'll admit, git + github tooling kind of forces a subpar workflow.

>>schind+(OP)
What one considers fast or slow may vary, but the general rule is something like the following.

  - very fast? run it all the time (shell prompt drawing, if you want, like direnv)
  - fast? run it in a pre-commit hook
  - a bit slow? run it in a pre-push book
  - really slow? run it in CI, during code review, etc.

Fwiw: I also rewrite history often-ish but it's never that fast for me because I have commit signing turned on and that requires verifying my presence to a USB smartcard on each commit. For me, it's okay if a commit takes a second or two. As it creeps up beyond 3 or 4 seconds, I become increasingly annoyed. If a commit took a minute I would consider that broken, and if I were expected to tolerate that or it were forced on me, I'd be furious.

I generally expect an individual pre-commit hook to under ~200ms (hopefully less), which seems reasonable to me. Some of the ones we have are kinda slow (more than 1s) and maybe should be moved to pre-push.

Since you seem especially sensitive to that latency, here's what I'd propose if we worked together:

If you own a repo, let's make all the hooks pre-push instead of pre-commit. On my repos, I like many hooks to run pre-commit. But since the hooks we use are managed by a system that honors local overrides via devenv.local.nix, let's make sure that's in .gitignore everywhere. When I'm iterating in your codebases and I want more automated feedback, I'll move more hooks to pre-commit, and when you're working in mine you can move all my hooks to pre-push (or just disable them while tidying up a branch).