Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Pre-commit hooks are broken

jyn.dev

Post reply on HN
Print view View original

51–60 of 178 posts

Re: Pre-commit hooks are broken

#51

Post by bawolff »

NekkoDroid wrote: post #49
bawolff wrote: post #35

Earlier quoted context omitted.

Honestly, i find that a really weird view. I use (Local) commits for work in progress. I feel like insisting on atomic commits in your local checkout defeats the entire purpose of using a tool like git. What do you do when you are working on something and are forced to switch to working on something else in the middle of it?

> What do you do when you are working on something and are forced to switch to working on something else in the middle of it? `git stash` is always an option :) but even if you want to commit it, you can always undo (or `--amend`) the commit when you get back to working. I personally am also a big fan of `git rebase -i` and all the things it allows me to fix up in the history before merging (rebasing) in to the main…

All of those are things i would refer to as making a commit :)

Re: Pre-commit hooks are broken

#52

Post by jynelson »

Dunedan wrote: post #44

The pre-commit framework [1] abstracts all these issues away and offers a bunch of other advantages as well. [1]: https://pre-commit.com/

the pre-commit framework does not abstract away “hooks shouldn’t be run during a rebase”, nor “hooks should be fast and reliable”, nor “hooks should never change the index”.

Re: Pre-commit hooks are broken

#53

Post by bawolff »

hbogert wrote: post #43
bawolff wrote: post #35

Earlier quoted context omitted.

Honestly, i find that a really weird view. I use (Local) commits for work in progress. I feel like insisting on atomic commits in your local checkout defeats the entire purpose of using a tool like git. What do you do when you are working on something and are forced to switch to working on something else in the middle of it?

I interpreted the parents post as: as long as my combination of commits results in something working before getting merged, it's fine. Local wip commits didn't come to mind at all

Well we are in a discussion about pre-commit hooks. Pre-commit hooks run on local wip commits.

Re: Pre-commit hooks are broken

#55

Post by burnt-resistor »

A workflow that works well is one that takes the better ideas from Meta's "hg"+"arcanist"+edenfs+"phabricator" diff and land strategy. Git, by itself, is too low-level for shared, mostly single-source-of-truth yet distributed dev.

Make test cases all green locally before pushing, but not in a way that interferes with pushing code and they shouldn't be tied to a particular (D)VCS. Allow uploading all of the separate proposed PRs you want in a proposed "for review" state. After a PR is signed-off and sent for merging, it goes into a linearizing single source of truth backed by an automated testing/smoke testing process before they land "auto-fast-forwarded" in a mostly uncontrolled manner that doesn't allow editing the history directly. Standardization and simplicity are good, and so is requiring peer review of code before it's accepted for existing, production, big systems.

Disallow editing trunk/main/master and whenever there's merge conflict between PRs, manual rebasing of one or the other is required. Not a huge deal.

Also, have structured OWNERS files that include people and/or distribution list(s) of people who own/support stuff. Furthermore, have a USERS file that keeps lists of people who would be affected by restarting/interrupting/changing a particular codebase/service for notification purposes too. In general, monorepo and allowing submitting code for any area by anyone are roughly good approaches.

Re: Pre-commit hooks are broken

#56

Post by tyleo »

dxdm wrote: post #50
PunchyHamster wrote: post #46

Earlier quoted context omitted.

> I want to add one other note: in any large organization, some developers will use tools in ways nobody can predict. This includes Git. Don't try to force any particular workflow, including mandatory or automatically-enabled hooks. you will save your org a lot of pain if you do force it, same as when you do force a formatting style rather than letting anyone do what they please. You can discuss to change it if some…

Enforcement should live in CI. Into people's dev environments, you put opt-in "enablement" that makes work easier in most cases, and gets out of the way otherwise.

Agreed, my company has some helper hooks they want folks to use which break certain workflows.

We’re a game studio with less technical staff using git (art and design) so we use hooks to break some commands that folks usually mess up.

Surprisingly most developers don’t know git well either and this saves them some pain too.

The few power users who know what they’re doing just disable these hooks.

Re: Pre-commit hooks are broken

#57

Post by xen0 »

wolfi1 wrote: post #22

why do people rebase so often? shouldn't it be excluded from the usual workflows as you are losing commit history as well?

Your real commit history is irrelevant. I don't care too much about how you came to a particular state.

The overall project history though, the clarity of changes made, and that bisecting reliably works are important to me.

Or another way; the important unit is whatever your unit of code review is. If you're not reviewing and checking individual commits, they're just noise in the history; the commit messages are not clear and I cannot reliably bisect on them (since nobody is checking that things build).

Re: Pre-commit hooks are broken

#58

Post by normie3000 »

hbogert wrote: post #25
normie3000 wrote: post #17

Earlier quoted context omitted.

> I don't want to commit something that doesn't build. This is a really interesting perspective. Personally I commit code that will fail the build multiple times per day. I only care that something builds at the point it gets merged to master.

so basically, not adhering to atomic commits. That's fine if it's a deliberate choice, but some people like me think commits should stand on their own. (i'm assuming your are not squashing when merging, else it's pretty much the same workflow)

> i'm assuming your are not squashing when merging, else it's pretty much the same workflow

I AM squashing before merging. Pre-commit hooks run on any commit on any branch, AFAIK. In any serious repo I'd never be committing to master directly.

Re: Pre-commit hooks are broken

#59

Post by lemonlime227 »

conradludgate wrote: post #19
lemonlime227 wrote: post #14

To bring up jujutsu, `jj fix` ( https://docs.jj-vcs.dev/latest/cli-reference/#jj-fix ) is a more refined way of ensuring formatting in commits. It runs a formatting command with the diff in stdin and uses the results printed to stdout. It can simplify merges and rebases history to ensure all your commits remain formatted (so if you enable a new formatting option, it can remove the need for a special format/style fix…

The downside currently (although I've been assured this will be fixed one day) is that it doesn't support running static analysis over each commit you want to fix. My git rebase workflow often involves running `git rebase -x "cargo clippy -- --deny=warnings"`. This needs a full checkout to work and not just a single file input

Yeah, to add some context for people reading this, jj fix works best for edits local to the diff, and it’s meant for edits mostly. With some trickery you could run some analysis, but it’s not what jj fix is meant for right now.

The intended future solution is `jj run` (https://docs.jj-vcs.dev/latest/design/run/), which applies similar ideas to more general commands.

Re: Pre-commit hooks are broken

#60

Post by seba_dos1 »

normie3000 wrote: post #16
nine_k wrote: post #7

A bit less enraged: pre-commit hooks should be pure functions. They must not mutate the files being committed. At best, they should generate a report. At worst, they could reject a commit (e.g. if it contains a private key file included by mistake).

> e.g. if it contains a private key file included by mistake Thanks - this is the first example of a pre-commit hook that I can see value in.

Remember that such key will be copied into the repository on `git add` already and will stay there until garbage collected.
Post reply on HN