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

31–40 of 278 posts

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

#31

Post by montag »

3eb7988a1663 wrote: post #17
imiric wrote: post #14

I tried using this tool after seeing recommendations for it, but IME it got the parent commit wrong enough times that the work to undo the damage was more than if I had looked up the commit myself and used `--fixup` instead. So I moved back to this manual workflow pretty quickly. I prefer having full control over my commit history, and this tool is too much magic for my taste. I'm sure that it could be improved so th…

I use lazygit for this. Keyboard driven TUI which lets you easily re-order/squash commits with minimum fuss. Being able to see them all laid out means no futzing with identifying the right ids.

This sounds just right.

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

#33

Post by boolemancer »

imiric wrote: post #20
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…

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 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?

2) I don't think I've ever cared about the context for a specific commit within a PR once the PR has been merged. What kind of information do you expect to get out of it?

Edit: How does it remove the context for a change or make `git bisect` useless? How big are your PRs that you can't get enough context from finding the PR commit to know why a particular change was made?

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

#34

Post by phist_mcgee »

imiric wrote: post #20
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…

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…

If you go all in on atomic commits, you're probably going to be rebasing a lot. IME, that's a lot of time spent trying to make a "clean" history all for the benefit of looking back infrequently at what broke.

Can't you just inspect your merged branches and bisect those instead if using the squash strat?

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

#35

Post by imiric »

hamandcheese wrote: post #30
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…

git bisect is still perfectly useful. It will locate the squashed commit/pr which is more than enough for me to zero in on whatever the bug is.

How helpful is it if it points you to a commit with 10 different changes? Will you go back to the PR to see the context of each change, and try to guess where the issue is based on the behavior? You're right back at manual debugging at that point.

This workflow really shines when it points you to the actual atomic commit that introduced the issue. Then you're simply a `git revert` away from undoing it, without risking breaking anything else, barring functional conflicts, of course. Try doing that with a squashed commit.

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

#36

Post by burntsushi »

The negativity in the comments here is unwarranted in my opinion. I've been using `git absorb` for years and it works amazingly well. I use it in addition to manual fixups. My most common uses of git-absorb, but definitely not the only, are when I submit a PR with multiple commits and it fails CI for whatever reason. If fixing CI requires changes across multiple commits (say, lint violations), then git-absorb will almost always find the right commit for each change automatically. It saves the tedium of finding the right commit for each change. False positives are virtually non-existent in my experience. False negatives do happen, but then you just fall back to the manual approach.

It seems like some would reply and say PRs should just be one commit. Or that they will be squashed anyway. And sure, that is sometimes the case. But not always. I tend to prefer logically small commits when possible, and it's not always practical to break them up across multiple PRs. Perhaps partially due to how GitHub works.

I use this workflow on all of my projects on GitHub.

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

#37

Post by burntsushi »

imiric wrote: post #35
hamandcheese wrote: post #30

Earlier quoted context omitted.

git bisect is still perfectly useful. It will locate the squashed commit/pr which is more than enough for me to zero in on whatever the bug is.

How helpful is it if it points you to a commit with 10 different changes? Will you go back to the PR to see the context of each change, and try to guess where the issue is based on the behavior? You're right back at manual debugging at that point. This workflow really shines when it points you to the actual atomic commit that introduced the issue. Then you're simply a `git revert` away from undoing it, without riskin…

I'm with you (see my other top level comment), but

> Then you're simply a `git revert` away from undoing it, without risking breaking anything else

This needs careful qualification. On GitHub at least, it is difficult to ensure every commit passes CI. This can result in skipping during bisect for a busted commit. It doesn't happen often enough in my experience too convince me to give up a cleaner history, but it's a downside we should acknowledge.

Ideally GitHub would make testing each individual commit easier.

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

#38

Post by recursive »

imiric wrote: post #35
hamandcheese wrote: post #30

Earlier quoted context omitted.

git bisect is still perfectly useful. It will locate the squashed commit/pr which is more than enough for me to zero in on whatever the bug is.

How helpful is it if it points you to a commit with 10 different changes? Will you go back to the PR to see the context of each change, and try to guess where the issue is based on the behavior? You're right back at manual debugging at that point. This workflow really shines when it points you to the actual atomic commit that introduced the issue. Then you're simply a `git revert` away from undoing it, without riskin…

I've not seen code where this revert thing would work. If it's in the PR something else now depends on it.

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

#39

Post by hoten »

a1o wrote: post #7

Uhm, I do a lot of git rebase -i HEAD~2 where I just squash the commit on the latest or sometimes I need to reorder and move the fix commits in specific commits in Pars that multiple commits, which I then need to push force. Is this for a similar use-case? I am not familiar with fixup or how it works.

fixup lets you mark a change to be rolled back into a previous commit, without changing the history (yet). When you do `git rebase --autosquash` it then automatically moves those commits as `fixup` in the right places. It helps to queue up many fixups at once and only have a single rebase to do when you're done (or for reviewers to see just the changes in a review tool, not have to reassess the entire commit because you already rebased)

btw you could be doing `git commit --amend` if you just want to add to the last commit.

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

#40

Post by recursive »

imiric wrote: post #20
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…

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…

There can be another reason. I do care about the benefits, but I don't think there really are any.
Post reply on HN