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

61–70 of 278 posts

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

#61

Post by gorset »

bradley13 wrote: post #56

Maybe I am being to much of a purist, but retroactively modifying commits and history? Why? Stuff happens, so do mistakes. Fix the mistakes, make another commit, and go on with your life.

The use case is when you look at a branch as a series of patches.

Reviewing a clean set of commits is much easier than a branch full of mistakes and wrong paths taken.

Useful when we optimize for reviewing and good history for future maintenance. This has been important and useful when I’ve worked on big mission critical backend system, but I also understand it might not be the most important factor for a new project’s success.

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

#62

Post by strunz »

bradley13 wrote: post #56

Maybe I am being to much of a purist, but retroactively modifying commits and history? Why? Stuff happens, so do mistakes. Fix the mistakes, make another commit, and go on with your life.

Commits should be a contained change that can be understood as a logical piece of history, and reverted if necessary. If you have to look at multiple commits for 1 logical change to the code, it's much more difficult to figured out what the intention was, if it was correct, and how it can be reverted.

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

#63

Post by notlinus »

strunz wrote: post #62
bradley13 wrote: post #56

Maybe I am being to much of a purist, but retroactively modifying commits and history? Why? Stuff happens, so do mistakes. Fix the mistakes, make another commit, and go on with your life.

Commits should be a contained change that can be understood as a logical piece of history, and reverted if necessary. If you have to look at multiple commits for 1 logical change to the code, it's much more difficult to figured out what the intention was, if it was correct, and how it can be reverted.

Do you always make all of your logical code changes in single, atomic commits? What if you have to modify a feature later on? I'm sorry, but this is ridiculous. There's nothing wrong with having multiple commits in a row modifying something. That's how git works. GitHub's PR merge workflow really messed up the meta game, I tell you what...

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

#64

Post by dclowd9901 »

gorset wrote: post #61
bradley13 wrote: post #56

Maybe I am being to much of a purist, but retroactively modifying commits and history? Why? Stuff happens, so do mistakes. Fix the mistakes, make another commit, and go on with your life.

The use case is when you look at a branch as a series of patches. Reviewing a clean set of commits is much easier than a branch full of mistakes and wrong paths taken. Useful when we optimize for reviewing and good history for future maintenance. This has been important and useful when I’ve worked on big mission critical backend system, but I also understand it might not be the most important factor for a new project…

So this wouldn’t work very well in workflows that flatten merges to a trunk?

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

#65

Post by nextaccountic »

bradley13 wrote: post #56

Maybe I am being to much of a purist, but retroactively modifying commits and history? Why? Stuff happens, so do mistakes. Fix the mistakes, make another commit, and go on with your life.

If it is unreleased it may make the life of reviewers easier. If they look at the commits at all, that is.

But if not even reviewers are looking at commits I question whether a PR should be chunked in commits at all - why not squash all commits into a single one, so that every PR is composed of exactly one commit? Could maybe separating the changes in commits convey some information?

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

#66

Post by notlinus »

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.

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

#67

Post by notlinus »

krick wrote: post #19
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…

Good for you, but you (and, apparently, everyone in your last 10 years of working with) would have a problem if I was the one reviewing your commits. I mean, to be fair, I often did let it slide (for political/social/practical reasons) and use autosquash, but I always actively discouraged it, so if you are a junior or a new hire with uncertain usefulness status, I'd at least talk to you about that and ask you to fix…

For what it's worth, you're right.

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

#68

Post by Zitrax »

masklinn wrote: post #50
burntsushi wrote: post #36

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 al…

I’ve been using autofixup for this and it’s been ok but not great, it can be quite slow as things grown, and it doesn’t say anything when there was no match so it’s easy to miss. How does absorb surface that? > Perhaps partially due to how GitHub works. That’s definitely a major factor, I’d like to use stacked PRs they sound really neat, but GitHub. Also even with stacked PRs I figure sometimes you’re at the top of t…

I assume you refer to https://github.com/torbiak/git-autofixup. I have also used it, and its ok but not perfect.

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

#69

Post by notlinus »

burntsushi wrote: post #36

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 al…

Criticism isn't negativity. We're not Pollyannas here, we're adults who can handle critique.

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

#70

Post by travisjungroth »

dclowd9901 wrote: post #64
gorset wrote: post #61

Earlier quoted context omitted.

The use case is when you look at a branch as a series of patches. Reviewing a clean set of commits is much easier than a branch full of mistakes and wrong paths taken. Useful when we optimize for reviewing and good history for future maintenance. This has been important and useful when I’ve worked on big mission critical backend system, but I also understand it might not be the most important factor for a new project…

So this wouldn’t work very well in workflows that flatten merges to a trunk?

It would mostly be unnecessary. The separate commits won’t matter after the PR if they’re getting squashed.

Debatably, if you’re making changes during a PR review, it could be helpful to make those changes in relevant commits. That way if someone goes through them during the PR, they get one clean “story”, rather than see the pre-PR commits and the conversation after.

Post reply on HN