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

Git-absorb: Git commit –fixup, but automatic

github.com

Post reply on HN
Print view View original

221–230 of 278 posts

Re: Git-absorb: Git commit –fixup, but automatic

#221

Post by travisb »

dpkirchner wrote: post #155
theptip wrote: post #142

Earlier quoted context omitted.

It sounds like you have never heard of the Stacked Diffs workflow. Here is an overview: https://newsletter.pragmaticengineer.com/p/stacked-diffs It’s a perfectly reasonable preference to eschew this workflow, but definitely worth understanding the pros and cons.

Every explanation of stacked diffs I've seen (including this one) makes me think there is some secret magic not being shared that will make the whole idea "click". If the problem is devs having to wait too long for PRs to be reviewed, breaking tasks up in to smaller diffs should exacerbate the problem, wouldn't it? If diff 2..n follow diff 1 then a review of 1 blocks everything else you're doing, eh? At least the tra…

In my experience with stacked diffs the smaller diffs doesn't exacerbate the problem because they are pipelined.

When getting merge approval takes at least one or two business days (say because of time zones or your colleagues are doing deep work which they don't want to drop every fifteen minutes to do a review) the pipelining is necessary to be unblocked.

Re: Git-absorb: Git commit –fixup, but automatic

#222

Post by burntsushi »

codetrotter wrote: post #220
burntsushi wrote: post #217

Earlier quoted context omitted.

> I make multiple commits in the PR and squash it all on merge. Sometimes I do that. But sometimes I want the history I've curated in my PR to be preserved without the overhead of creating separate PRs (which then often need to be stacked, introducing problems of its own). In which case, I avoid things like "fixup lint" commits. And that's where git-absorb shines. And saving time is exactly the point! git-absorb help…

> That's a lot more work than just reading the commit log. Yeah, that’s true. I suppose that we may be working in somewhat different environments. For example, when someone has big public projects that get a lot of eyeballs on them, like your ripgrep and other projects, it makes a lot of sense to spend extra time making the git log a thing that can be read on its own completely offline. For the things I work on at my…

Yeah folks work differently. I'm just mostly responding to this idea that (roughly paraphrasing) "hey we don't need this tool, you should just be squash merging instead."

FWIW, at my previous role, where I worked on an internal proprietary codebase, we all followed this same philosophy of curating commits. It was primarily to facilitate code review though, and not source history. Stacking PRs on GitHub is really truly annoying in my experience. (Although there is some tooling out there that purports to improve the experience.)

Re: Git-absorb: Git commit –fixup, but automatic

#223

Post by WorldMaker »

homebrewer wrote: post #176
menaerus wrote: post #162

Earlier quoted context omitted.

What is wrong with simply pushing a "Fix linting issues" in a new commit? It's self-contained and very well describes the (single) purpose of the commit. I share the sentiment about the "logical small commits", and hence I don't see that adding a new fix commit is problematic as long as it is self-contained and purposeful, but perhaps I don't understand what is the problem that this tool is trying to solve. It says >…

It makes git bisect more difficult than it needs to be.

An option is `git bisect --first-parent` to start from your integration points. (Then drill down into that branch if you need to.)

Re: Git-absorb: Git commit –fixup, but automatic

#224

Post by titusjohnson »

enw wrote: post #158

In what situations does this solve a problem? In our projects we only enable squash merge in GitHub and the PRs can have any commits you want. The squashed commit includes link to PR, and PR has detailed summary (which wouldn’t be practical in the commit message).

Agreed. It's been a decade or more since I and the teams I worked with did anything other than squash-merging a feature branch into main. The PR body becomes the commit body for the squash, and we're done.

Developers can be as f'n messy as they want on their branches, no one cares because it never hits main.

Re: Git-absorb: Git commit –fixup, but automatic

#225

Post by keybored »

boolemancer wrote: post #33
imiric wrote: post #20

Earlier quoted context omitted.

Every team is free to choose what works best for them, but IMO always squashing PRs is not a good strategy. Sometimes you do want to preserve the change history, particularly if the PR does more than a single atomic change, which in practice is very common. There shouldn't be a static merge type preference at all, and this should be chosen on a case-by-case basis. At the risk of sounding judgemental, I think this pre…

> At the risk of sounding judgemental, I think this preference for always squashing PRs comes from a place of either not understanding atomic commits, not caring about the benefits of them, or just choosing to be lazy. In any case, the loss of history inevitably comes at a cost of making reverting and cherry-picking changes more difficult later, as well as losing the context of why a change was made. 1) Why are you e…

> Why are you ever reverting/cherry-picking at a more granular level than an entire PR anyway? The PR is the thing that gets signed-off on, and the thing that goes through the CI build/tests, so why wouldn't that be the thing kept as an atomic unit?

The PR could contain three refactoring commits and one whitespace cleanup commit. One main change. The only thing that breaks anything is the main change. Because those others were no-ops as far as the code is concerned.

Re: Git-absorb: Git commit –fixup, but automatic

#226

Post by keybored »

imiric wrote: post #93
boolemancer wrote: post #33

Earlier quoted context omitted.

> At the risk of sounding judgemental, I think this preference for always squashing PRs comes from a place of either not understanding atomic commits, not caring about the benefits of them, or just choosing to be lazy. In any case, the loss of history inevitably comes at a cost of making reverting and cherry-picking changes more difficult later, as well as losing the context of why a change was made. 1) Why are you e…

> The PR is the thing that gets signed-off on, and the thing that goes through the CI build/tests, so why wouldn't that be the thing kept as an atomic unit? Because it often isn't. I don't know about your experience, but in all the teams I've worked in throughout my career the discipline to keep PRs atomic is almost never maintained, and sometimes just doesn't make sense. Sometimes you start working on a change, but…

> I find conventional commits helpful when deciding what makes an atomic change.

I already know if I’m doing a fix, a refactor, a “chore” etc. Conventional commits just happen to be the ugliest way you can express those “types” in what looks like English.

Re: Git-absorb: Git commit –fixup, but automatic

#227

Post by keybored »

seadan83 wrote: post #48
imiric wrote: post #20

Earlier quoted context omitted.

Every team is free to choose what works best for them, but IMO always squashing PRs is not a good strategy. Sometimes you do want to preserve the change history, particularly if the PR does more than a single atomic change, which in practice is very common. There shouldn't be a static merge type preference at all, and this should be chosen on a case-by-case basis. At the risk of sounding judgemental, I think this pre…

What are your thoughts on the "ship, show, ask" workflow? [1] In that workflow, small stuff is simply pushed, which allows PRs to be more single focused and more atomic. Perhaps your only objection is direct pushes to master? I am really curious if that workflow otherwise addresses all of the downsides you stated while still allowing for all PRs to be uniformly rebase-squash merged. [1] https://martinfowler.com/artic…

Why is the preferred goal to “rebase-squash” (redundant) merge? The implied onus here seems to be for the other party to move towards that strategy. But why this practice should be the goal does not seem to be mentioned.

Step one isn’t to find some way to accept squash-merge. Step one is to find some reason for squash-merge.

Re: Git-absorb: Git commit –fixup, but automatic

#228

Post by metadat »

I haven't used `git --fixup' or `git rebase --autosquash' before, but they sound pretty handy.

https://jordanelver.co.uk/blog/2020/06/04/fixing-commits-wit...

git-absorb appears to take it one level further. However from the README I'm not clear on exactly what it will do in specific situations:

Does git-absorb automatically associate the most recent commit unique to a branch for a given file and apply the diff to said commit? Or what is the precise workflow and outcome in terms of which edits go into which commits?

Re: Git-absorb: Git commit –fixup, but automatic

#229

Post by keybored »

notlinus wrote: post #66
lr4444lr wrote: post #13

you don't want to shove them all into an opaque commit that says fixes, because you believe in atomic commits. Sure I do. The whole branch will be squashed anyway before it's merged in, and a single "fixes" commit while still on its own branch will be easier to track in a PR for addressing everything pointed out earlier. I mean, don't let me stop anyone from using this or --fixup if this is your flow, but this solves…

I wish people would stop saying "atomic commits" and start saying "main/master is stable", because that's what they actually mean. Every git commit is atomic, by definition... But people want every single possible revision to be green and buildable, which is different, and has nothing to do with git. I don't think it makes sense (tags are a lot more helpful for marking what is and isn't stable), but hey.

People say what they mean.

“Atomic” sometimes means conceptually: does one thing. Sometimes it means “builds”. Or “works”. And yes: they do mean every single commit builds. Not just on main.

You don’t have to put words in people’s mouth.

Re: Git-absorb: Git commit –fixup, but automatic

#230

Post by keybored »

dmead wrote: post #42

This sounds great,but kind of an anti pattern in git. I definitely want to have a "fixes" commit on my feature branch. You should do whatever you want on a feature branch so long as your trunk has a clean history. This sounds like someone wanted to lift a feature of changesets in mercurial into git. I don't think this is safe and probably breaks a lot of people's mental model of git changelogs being an immutable data…

> I definitely want to have a "fixes" commit on my feature branch. You should do whatever you want on a feature branch so long as your trunk has a clean history.

This tool creates `fixup!` commits. These commits aren’t rewrite commits. They are commits which later can be used to rewrite the history.

You can use this tool, not rewrite your branch, and then rewrite the branch right before merge.

> This sounds like someone wanted to lift a feature of changesets in mercurial into git. I don't think this is safe and probably breaks a lot of people's mental model of git changelogs being an immutable data structure.

Rewriting is a huge part of the Git culture (and well-supported). Meanwhile Mercurial seems to have rewriting as a sort of add-on.

Post reply on HN