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

121–130 of 178 posts

Re: Pre-commit hooks are broken

#121

Post by johnisgood »

jghn wrote: post #118
tharkun__ wrote: post #101

Earlier quoted context omitted.

My branch is mine. Don't tell me what I can or can't do. I push WIP stuff all the time, to share code with others for discussion, to get the build to run in parallel while I keep working or just at the end of the day. I freely amend and will squashed before merging (we only allow a single commit per branch to go to master). If I or someone else bases something off anything but master that's on them to rebased and kee…

My philosophy is that once a PR is open, that's the point at which people should no longer feel free to treat their branch as their own. Even in groups that squash commits, it should still preserve the aggregate commit messages. But until that PR is open? Totally with you. There is no obligation to "preserve history" up until that point.

Not to disagree, but this is so GitHub-centric. What is up with "diffs", "patches", and "submissions"? :D

Re: Pre-commit hooks are broken

#122

Post by jghn »

johnisgood wrote: post #121
jghn wrote: post #118

Earlier quoted context omitted.

My philosophy is that once a PR is open, that's the point at which people should no longer feel free to treat their branch as their own. Even in groups that squash commits, it should still preserve the aggregate commit messages. But until that PR is open? Totally with you. There is no obligation to "preserve history" up until that point.

Not to disagree, but this is so GitHub-centric. What is up with "diffs", "patches", and "submissions"? :D

Not to disagree, but calling it Github-centric is a bit over specific :)

I regularly work with Github, Bitbucket, and Gitlab. Everything I said applies except for the fact that I said "PR" instead of "MR". But yes, you're right. I'm highlighting a specific, albeit extremely popular, workflow.

Re: Pre-commit hooks are broken

#123

Post by badgersnake »

thomashabets2 wrote: post #65
bawolff wrote: post #53

Earlier quoted context omitted.

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

Well, unless you inhibit them with `-n`. Which I would for WIP commits.

Then what’s the point? Just leave them off and run the tests when you want to run them.

Re: Pre-commit hooks are broken

#124

Post by ltbarcly3 »

hxtk wrote: post #112
ltbarcly3 wrote: post #103

Earlier quoted context omitted.

This is not how code is actually written.

It's how code is written in Google (including their open-source products like AOSP and Chromium), the ffmpeg project, the Linux Kernel, Git, Docker, the Go compiler, Kubernetes, Bitcoin, etc, and it's how things are done at my workplace. I'm surprised by how confident you are that things simply aren't done this way considering the number of high-profile users of workflows where the commit history is expected to tell…

"It's how code is written" then you list like the 6 highest profile, highest investment premier software projects on Earth like that's just normal.

I'm surprised by how confident you are when you can only name projects you've never worked on. I wanted to find a commit of yours to prove my point, but I can't find a line of code you've written.

Re: Pre-commit hooks are broken

#125

Post by johnisgood »

jghn wrote: post #122
johnisgood wrote: post #121

Earlier quoted context omitted.

Not to disagree, but this is so GitHub-centric. What is up with "diffs", "patches", and "submissions"? :D

Not to disagree, but calling it Github-centric is a bit over specific :) I regularly work with Github, Bitbucket, and Gitlab. Everything I said applies except for the fact that I said "PR" instead of "MR". But yes, you're right. I'm highlighting a specific, albeit extremely popular, workflow.

I know, I know, I was going to edit it to "Git{Hub,Lab}" in the beginning but oh well.

In any case, my comment just reflects on the fact that you had a series of patches that you could not squash or rebase. It stuck.

And the fact that I see many people use the abbreviation "PR" for something that is merely a patch or diff. For example you might send a diff to the tech@ mailing list, but you should not refer to it as a PR.

Re: Pre-commit hooks are broken

#126

Post by QuercusMax »

mcv wrote: post #116
nrclark wrote: post #2

This was a really interesting read. I'd highly recommend it for anybody who's setting up (or currently maintains) a pre-commit workflow for their developers. 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. Instead, put what you want in a…

There's a weird thing happening on my current project. Sometimes I merge main into my branch and it fails. What fails is the pre-commit hook on the merge commit. Changes in main fail the linting checks in the pre-commit hook. But they still ended up in main, somehow. So the checks on the PR are apparently not as strict as the checks on the pre-commit hook. As a result, many developers have gotten used to committing w…

Your hook really shouldn't be running on the merge commit unless you have conflicts in your merge.

Re: Pre-commit hooks are broken

#127

Post by QuercusMax »

johnisgood wrote: post #125
jghn wrote: post #122

Earlier quoted context omitted.

Not to disagree, but calling it Github-centric is a bit over specific :) I regularly work with Github, Bitbucket, and Gitlab. Everything I said applies except for the fact that I said "PR" instead of "MR". But yes, you're right. I'm highlighting a specific, albeit extremely popular, workflow.

I know, I know, I was going to edit it to "Git{Hub,Lab}" in the beginning but oh well. In any case, my comment just reflects on the fact that you had a series of patches that you could not squash or rebase. It stuck. And the fact that I see many people use the abbreviation "PR" for something that is merely a patch or diff. For example you might send a diff to the tech@ mailing list, but you should not refer to it as…

Git{Hu,La}b

Re: Pre-commit hooks are broken

#128

Post by seniorsassycat »

a_t48 wrote: post #29

Running on the working tree is mostly okay - just `exit 1` if changes were made and allow the user to stage+commit new changes. It isn't perfect but it doesn't require checking out a new tree.

What if I've already fixed the format issue (but not staged it). The pre-commit hook will pass, but it's not doing what the author intended (preventing unformated code from being committed).

What if I've only staged one part of a file, but the pre-commit hook fails on the unstaged portions, which should be fine since I'm not commiting or pushing those changes.

Re: Pre-commit hooks are broken

#129

Post by nirvdrum »

jghn wrote: post #119
nirvdrum wrote: post #86

Earlier quoted context omitted.

If you’re just committing for your own sake, that workflow sounds productive. I’ve been asked to review PRs with 20+ commits with a “wip” or “.” commit message with the argument: “it’ll be squash merged, so who cares!”. I’m sure that works well for the author, but it’s not great for the reviewer. Breaking change sets up into smaller logical chunks really helps with comprehension. I’m not generally a fan of people bei…

There are two halves here. Up until the PR is open, the author should feel free to have 20+ "wip" commits. Or in my case "checkpoint". However, it is also up to the author to curate their commits before pushing it and opening the PR. So when I open a Pr, I'll have a branch with a gajillion useless commits, and then curate them down to a logical set of commits with appropriate commit messages. Usually this is a single…

I clean up commits locally as well. But, I really only commit when I think I have something working and then collapse any lint or code formatting commits from there. Sometimes I need to check another branch and am too lazy to set up worktrees, so I may create a checkpoint commit and name it a way that reminds me to do a `git reset HEAD^` and resume working from there.

But, if you're really worried about losing 15 minutes of work, I think we have better tools at our disposal, including those that will clean up after themselves over time. Now that I've been using ZFS with automatic snapshots, I feel hamstrung working on any Linux system just using ext4 without LVM. I'm aware this isn't a common setup, but I wish it were. It's amazing how liberating it is to edit code, update a config file, install a new package, etc. are when you know you can roll back the entire system with one simple command (or, restore a single file if you need that granularity). And it works for files you haven't yet added to the git repo.

I guess my point is: I think we have better tools than git for automatic backups and I believe there's a lot of opportunity in developer tooling to help guard against common failure scenarios.

Re: Pre-commit hooks are broken

#130

Post by ErenayDev »

nirvdrum wrote: post #129
jghn wrote: post #119

Earlier quoted context omitted.

There are two halves here. Up until the PR is open, the author should feel free to have 20+ "wip" commits. Or in my case "checkpoint". However, it is also up to the author to curate their commits before pushing it and opening the PR. So when I open a Pr, I'll have a branch with a gajillion useless commits, and then curate them down to a logical set of commits with appropriate commit messages. Usually this is a single…

I clean up commits locally as well. But, I really only commit when I think I have something working and then collapse any lint or code formatting commits from there. Sometimes I need to check another branch and am too lazy to set up worktrees, so I may create a checkpoint commit and name it a way that reminds me to do a `git reset HEAD^` and resume working from there. But, if you're really worried about losing 15 min…

I'm thinking of writing a tool related to the "checkpoint" system when I have some free time. Do you have any advices?
Post reply on HN