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

151–160 of 178 posts

Re: Pre-commit hooks are broken

#151

Post by ltbarcly3 »

Izkata wrote: post #117
ltbarcly3 wrote: post #99

Earlier quoted context omitted.

I agree, in a job where you have no documentation and no CI, and are working on something almost as old or older than you with ancient abandoned tools like svn that stopped being relevant 20 years ago, and in a fundamentally dysfunctional company/organization that hasn't bothered to move off of dead/dying tools in the last 20 years, then you just desperately grab at anything you can possibly find to try to avoid brea…

It is not possible to manually document everything down to individual lines of code. You'll drive yourself crazy trying to do so (and good luck getting anyone to look at that massive mess), and that's not even counting how documentation easily falls out of date. Meanwhile, we have "git blame" designed to do exactly that with almost no effort - just make good commit messages while the context is in your head. CI also…

> Calling svn part of the problem is also kind of backwards - it has no bearing on the code quality itself

Lets just be Bayesian for a minute. If an organization can't figure out how to get off of svn, which is a dead and dying technology within 15-20 years of it being basically dead in most of tech then probably it's not not going to be nimble in other ways. Probably it's full of people who don't really do any work.

> Some of the internal wiki was migrated directly to Confluence (and lost all formatting and internal links in the process)

Dude this is what I mean. How did someone manage to mess this up? It's not exactly rocket science to script something to suck out of one wiki and shove into another one. But lets say it's hard to do (it's not). Did they just not even bother to look at what they did? They just figured "meh" and declared victory and then three were no consequences, nobody bothered to go back and redo it or fix it? Moving stuff between wiki's is an intern-skill-level task. This is another example that screams that the people at your work don't do their jobs and don't care about their work, and that this is tolerated or more likely not even noticed. Do you work for the government?

> Commit messages are really the only thing you can actually rely on.

I suspect you are exaggerating how reliable your commit messages are, considering.

Re: Pre-commit hooks are broken

#152

Post by thomashabets2 »

normie3000 wrote: post #142
thomashabets2 wrote: post #136

Earlier quoted context omitted.

Because 99% of my commits are not WIP commits. So I almost always want to run them. Hell, even most WIP commits will pass the tests (e.g. tests are not yet added for the new code), so I'd run them then too.

Some people write tests first.

And commit in such that the final timeline has broken tests for half of commits?

Sounds like an awful way to live your life.

Re: Pre-commit hooks are broken

#153

Post by drio »

amluto wrote: post #134

Home Assistant takes this one step farther. There is a pre- run hook that goes out its way to make it hard to run an “integration” that doesn’t meet its quality standard. I get that they don’t want to get PRs for integrations that don’t check the checklist, but as someone writing an integration (which I’m currently doing, for better or for worse), I want to run my own incomplete integration, thank you very much. (One…

Curious to hear what integration you are working on.

I am thinking about making this (1) into an integration also. It is an alternative way to visualize forecasts. It uses the dew point instead of temperature. I currently use a dashboard that shows the site on an iframe within Home Assistant.

1. https://observablehq.com/@drio/shader-galaxy

Re: Pre-commit hooks are broken

#154

Post by amluto »

drio wrote: post #153
amluto wrote: post #134

Home Assistant takes this one step farther. There is a pre- run hook that goes out its way to make it hard to run an “integration” that doesn’t meet its quality standard. I get that they don’t want to get PRs for integrations that don’t check the checklist, but as someone writing an integration (which I’m currently doing, for better or for worse), I want to run my own incomplete integration, thank you very much. (One…

Curious to hear what integration you are working on. I am thinking about making this (1) into an integration also. It is an alternative way to visualize forecasts. It uses the dew point instead of temperature. I currently use a dashboard that shows the site on an iframe within Home Assistant. 1. https://observablehq.com/@drio/shader-galaxy

Lutron QS Standalone, to control Lutron devices via the QSE-CI-NWK-E.

Re: Pre-commit hooks are broken

#155

Post by mcv »

QuercusMax wrote: post #126
mcv wrote: post #116

Earlier quoted context omitted.

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.

Never had conflicts on a merge? We've got a lot of people on the same codebase. Merge conflicts are a fact of life. And they wouldn't be a problem without the stupid commit hook. It's the commit hook that makes them a problem.

Re: Pre-commit hooks are broken

#156

Post by ltbarcly3 »

nirvdrum wrote: post #110
ltbarcly3 wrote: post #96

Earlier quoted context omitted.

> A well laid out history of logical changes makes reviewing complicated change sets easier. Rather than one giant wall of changes, you see a series of independent, self contained, changes that can be reviewed on their own. But this would require hand curation? No development proceeds that way, or if it does then I would question whether the person is spending 80% of their day curating PRs unnecessarily. I think you…

> But this would require hand curation? No development proceeds that way, or if it does then I would question whether the person is spending 80% of their day curating PRs unnecessarily. It's not really hand curation if you're deliberate about it from the get-go. It's certainly not eating up 80% of anyone's time. Structuring code and writing useful commits a skill to develop, just like writing meaningful tests. As a f…

Aha, I see the issue here. What you seem to organize into cute little self contained 'commit's I would put on individual 'branches'.

It is too hard for you to get someone to look at a PR, so you are packing multiple 'related' but not interdependent changes into one PR as individual commits so you can minimize the number of times you have to get someone to hit "approve", which is the limiting resource.

In your situation then I believe your way of working is a rational adaptation, but only so far as you lack the influence to address the underlying organizational/behavioral dysfunction. We agree on the underlying need to make good messages, but where I merge 4-5 small branches per day, each squashed to one commit, you are saving them all up to get them (unnecessarily) put into a single merge commit.

Just as "Structuring code" is a skill to develop, so is building healthy organizations.

Re: Pre-commit hooks are broken

#158

Post by WorldMaker »

sunshowers wrote: post #148
cluckindan wrote: post #141

Earlier quoted context omitted.

I’m sure you’re here to educate me, but this is not about criss-cross merges between two different work branches, this is about whether it’s better to rebase a work branch onto the main branch, or to pull the changes from the main branch to the work branch.

I have an early draft of a blog post about them :) as a source control expert who built both these systems and tooling on top of them for many years, I think they're the biggest and most fundamental reason rebases/linear history are better than merges. > whether it’s better to rebase a work branch onto the main branch, or to pull the changes from the main branch to the work branch. The problem with this is that the l…

It's definitely not 0 because rebase heavy workflows involve the rerere cache which is a minefield of per-repo hidden merge changes. You get the results of "criss-cross merges" as "ghosts" you can't easily debug because there aren't good UI tools for the rerere cache. About the best you can do is declare rerere cache bankruptcy and make sure every repo clears their rerere cache.

I know that worst case isn't all that common or everyone would be scared of rebases, but I've seen it enough that I have a healthy disrespect of rebase heavy workflows and try to avoid them when given the option/in charge of choosing the tools/workflows/processes.

Re: Pre-commit hooks are broken

#159

Post by andrewaylett »

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.

Strong disagree: until the branch is merged, it's mine.

I'm in a camp that prefers single rebased commits as units of change, "stacked diffs" style.

GitHub in particular was annoying with this style but is definitely getting better. It's still not great at dealing with actual stacks of diffs, but I can (and do) work around that by keeping the stack locally and only pushing commits that apply directly to the main branch.

Re: Pre-commit hooks are broken

#160

Post by andrewaylett »

ErenayDev wrote: post #130
nirvdrum wrote: post #129

Earlier quoted context omitted.

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?

I think you might appreciate https://www.jj-vcs.dev, which makes it a lot easier to split and recombine changes. I often use it for checkpoints, although you wouldn't see that from looking at what I push :).

One nifty feature is that commits don't need messages, and also it'll refuse (by default) to push commits with no message. So your checkpoint commits are really easy to create, and even easier to avoid pushing by mistake.

Post reply on HN