Live data from Hacker News

Git rebase, what can go wrong

jvns.ca

91–100 of 404 posts

Re: Git rebase, what can go wrong

#91

> force pushing makes code reviews harder On any code base I've worked on that's larger than a small FOSS project, I've found that this simply isn't avoidable. Yes, there's merge commits but, for reasons I won't go into, I think those are worse than the alternative of rebasing and making code reviews difficult. > One way to avoid this is to push new commits addressing the review comments, and then after the PR is app…

> Not realistic when working on a code base where PRs are being squash-merged every hour and the code review lasts for days.

But if they collide, you have to resolve all the merge conflicts anyway, and then with rerere you should have relatively little additional work on the final rebase.

Re: Git rebase, what can go wrong

#92
post #2

I like how Atlassian puts it: > The golden rule of rebasing > Once you understand what rebasing is, the most important thing to learn is when not to do it. The golden rule of git rebase is to never use it on public branches. https://www.atlassian.com/git/tutorials/merging-vs-rebasing#... For me, even though rebasing comes with some trappings, I still greatly prefer it to the alternative, which is to have merge commit…

Squash merges cut down the noise considerably.

I think squash merges are a last resort heavy-handed tool for dealing with developers who refuse to clean up their commit history before merging. Most developers can do better by hand.

Git history should tell a simple, understandable story of each change. For example: 1) refactor existing code, 2) add feature. Or 1) add missing tests, 2) refactor existing code, 3) add feature.

But since you're working on the fly with imperfect knowledge, it doesn't happen in such neat steps. Refactorings and behavior changes end up interleaved in your raw git history, so you need to do a little bit of cleanup by hand in order to present a simple story in the commit log.

Of course if you have developers that don't do that and instead merge dozens of commits that just say wip, wip, wip, lol, fml, wip, wip, lol, yolo and you can't fire them or get them to change, then squash merges ftw.

Re: Git rebase, what can go wrong

#93

Uising squash merges has reduced my need to rebase a lot. I don't really care if I have merge commits on a feature branch for a PR if there's a reasonable history on the main branch when I'm troubleshooting an issue with git blame. Why squash merges? I have a number of team-mates who make local commits on feature branches that make the history look like a series of less-than-useful commit messages. E.g. wip, wip, wip…

Squash merges are rebases.

Re: Git rebase, what can go wrong

#95
post #8

I love rebase (I'm a tip-of-master-only person, no merges ever, squash all your commits with `rebase -i` before pushing and write one good commit message for the group). But there's one really, really irritating thing about them: You should not be able to use `--amend` during a rebase. For me editing all my changes onto the commit I'm working on with `git commit -a --amend` (or as I've aliased it, `gcaa`) is automati…

> accidentally typed gcaa and amended someone else's commit after fixing a merge conflict

You could try reverting the first commit on the HEAD once you finish the rebase. This is of course assuming your branch and the last commit don't touch the same files.

Re: Git rebase, what can go wrong

#96
post #63

Earlier quoted context omitted.

This is the big one for me. destroying Commit information just to keep the graph tidy is a bad idea in my opinion. It would be better if Git provided better tools for filtering the log, e.g. providing some mechanism to elide commits from parents of any merge commit other than the 1st.

> destroying Commit information just to keep the graph tidy is a bad idea in my opinion The commit information I see when telling teams to squash their branches on merge is not valuable. * "fixing whitespace" * "incorporate review comments" * "fix broken test" * "fix other broken test" (note, the broken tests were broken by the changes in the PR) As soon as that PR is merged those commits are worthless. And there are…

Yep, intermediate commits on a branch tend to be completely worthless. I'd much rather have "git blame" point to the commit that contains the entire change together.

Re: Git rebase, what can go wrong

#97
I lost patience with the various git commit cleanup tools and now I just go nuclear. I use git diff > output.file, make a new branch, get apply output.file.

Fresh clean branch, no commit history, create pull request.

I'm not convinced there's any value to incremental commit messages. This simple, clean, and undoable as long as I keep my initial branch

Re: Git rebase, what can go wrong

#98
post #63

Earlier quoted context omitted.

> destroying Commit information just to keep the graph tidy is a bad idea in my opinion The commit information I see when telling teams to squash their branches on merge is not valuable. * "fixing whitespace" * "incorporate review comments" * "fix broken test" * "fix other broken test" (note, the broken tests were broken by the changes in the PR) As soon as that PR is merged those commits are worthless. And there are…

> * "fixing whitespace" * "incorporate review comments" * "fix broken test" * "fix other broken test" Things like this should not be standalone commits though, they should be incorporated into the previous branch by amending the original work. It takes some effort to have a useful git history, it does not just happen on its own.

Sounds like six vs half-dozen. Why does it matter if somebody amends vs squashes?

Re: Git rebase, what can go wrong

#100
post #63

Earlier quoted context omitted.

> destroying Commit information just to keep the graph tidy is a bad idea in my opinion The commit information I see when telling teams to squash their branches on merge is not valuable. * "fixing whitespace" * "incorporate review comments" * "fix broken test" * "fix other broken test" (note, the broken tests were broken by the changes in the PR) As soon as that PR is merged those commits are worthless. And there are…

Those commits would be the bathwater one casts out alongside the useful commits in using squash merges.

If the useful commits are the "baby" in your bathwater analogy, all the useful information in those commits is in the squashed commit.

This assumes a branch being merged in represents one logical change (a feature/bugfix/etc) that is "right sized" to be represented by one commit.

Post reply on HN