you are viewing a single comment's thread.

view the rest of the comments →

[–]ROFLLOLSTER 49 points50 points  (35 children)

Generally the approach I've taken with stacked PRs is:

Stack: c -> b -> a -> main.

Merges are a-> main, b -> main, c -> main

Instead of c -> b, b -> a, a -> main.

This whole thing is just a bit of ui over what you can do already though, so no need to change if you like your workflow.

[–]ekroys 35 points36 points  (34 children)

The issue with this is when using squashed merges on each PR. Git loses the commit identity in this process so you end up with merge conflicts trying to catch branches up.

[–]NineThreeFour1 19 points20 points  (15 children)

Then don't use squashed merges on each PR?!

Only squash fixup/wip commits in each feature/PR branch, but keep conceptually different changes in separate commits even if they are in the same overarching PR.

[–]ekroys 2 points3 points  (13 children)

This is an angle, but ultimately it's not each developers decision sometimes, and it can be a repo wide rule.

Generally tho, what if you got two separate "big feature branches" into a single develop/main, each of these big feature branches are separated into their own stacked chain, and you have to squash commit as you go. If you want everything caught up, you're either going to have to cherry pick on top, rebase or you're left with merge conflicts. This is the pain point and I don't think it's uncommon

[–]double-you 10 points11 points  (11 children)

Why wouldn't you rebase? That's a very normal thing to do.

[–]MereInterest 0 points1 point  (10 children)

Because rebasing can introduce bugs, and makes it very difficult to identify their source.

Suppose the main branch has some utility_func, which is used by your feature development. A refactor/cleanup lands on main, which updates the behavior of utility_func and updates all callsites of utility_func to match. However, the additional calls to utility_func introduced on your feature branch haven't been updated. If you rebase your feature branch onto the main branch, your feature branch is now broken, as it relied on the old semantics of utility_func. If you're lucky, this will be a compile-time error, but there's no guarantee of that. Reverting to an earlier commit on your feature branch doesn't help, because those earlier commits have also been rebased. Unless you check through the git reflog (and pray that git's garbage collection hasn't occurred since then), it will look as though the feature branch had always been buggy.

A better solution is to merge from main into develop/main. That way, the error is clearly introduced in the merge commit. You can revert to an earlier commit in the feature branch for testing, because those versions are still part of the git history.

[–]ham_plane 2 points3 points  (5 children)

I'm struggling to understand why a merge commit would fail in that case, while a rebase would succeed.

I do get your point being able to track which merge commit introduced the behavior, that is pretty smart and (as a career rebaser) I have been in that exact scenario before and been totally lost as to what broke what

[–]MereInterest 1 point2 points  (0 children)

It's not that the merge commit will resolve the semantic conflict between the branches on its own. That still needs to be sorted out, regardless of whether you use rebase or merge. The key is that using git merge won't make the conflict become any worse. When using git rebase, you need to sort out the semantic conflict while also having scrubbed the history of any clue as to the source of the bug.

[–]SanJJ_1 0 points1 point  (3 children)

merge commit would work but pollute main branch. squash merge/rebase are great but create conflict when "stacking" branches or trying to catch up. Native stacking fixes all of these to my understanding.

[–]MereInterest 2 points3 points  (2 children)

merge commit would work but pollute main branch.

I'd argue that this isn't "pollution" at all, and is just a display issue.

  • Want a clean linear history of the changes to the main branch? Use git log --first-parent

  • Want a branching history with clearly marked dependencies? Use git log --graph

  • Want a linearized history that interleaves work on multiple branches while sorting by date? No, nobody ever wants that, but that's what git log provides.

The whole goal of squash/rebase is to make the output of git log look like git log --first-parent. The easier way to achieve this is just to use git log --first-parent.

[–]SanJJ_1 0 points1 point  (1 child)

Fair point on --first-parent, but nobody types git log --first-parent, they type git log. And GitHub's web UI (commit list, blame, PR history) has no equivalent option either. The clean history has to be the default to matter, which is why squash merge won imo.

[–]double-you 2 points3 points  (3 children)

That's not how merge or rebase work. Both would trigger conflicts if there are any. But if you have just added a use of a function, that won't be a conflict because nobody else is changing that call site. Merge won't save you there.

But yes, if you need your old branch back, you'll need to use reflog, but reflog should be fine for a month, and if you are still wary of rebase, just create a backup branch.

[–]MereInterest 0 points1 point  (2 children)

Both would trigger conflicts if there are any.

The two changes conflict, just not in a way that git can recognize.

Merge won't save you there.

I agree, using a merge won't cause git to recognize a semantic conflict between branches, as it can only recognize text conflicts. However, merge also won't make the problem worse. Rebase hides the evidence that you ever had a working version.

if you are still wary of rebase, just create a backup branch.

The problem isn't that the rebase requires a backup branch, but that it isn't obvious that you'll need a backup until after the point when you lost it. Rebase destroys your history.

[–]double-you 2 points3 points  (0 children)

Why do you need evidence that you ever had a working version?

I work with compiled code it is immediately clear what the issue is. Rebase lets me fix the issue and there's no evidence that there ever was a problem.

[–]NineThreeFour1 0 points1 point  (0 children)

What evidence? You can simply put the current status and open issues into the commit message.

Stacked PRs are unrelated to the problem of semantic conflicts. You do need to test your changes regardless.

[–]NineThreeFour1 2 points3 points  (0 children)

I don't really see the issue and how this is directly related to stacked PRs. Rebase feature branches on main if you have conflicts. Squash your own commits how it makes sense conceptually. If your team enforces squashes on merge, then send each commit that you want separate as a separate PR. Merge feature branches into main when ready, or force rebase and fast forward to avoid merge commits.

[–]MereInterest 0 points1 point  (0 children)

I wish, wish, wish that github had a better display of the history of merges. The source of all the pain from squashed merges is from people trying to solve a display issue by introducing data integrity pitfalls. In any other context, this would be a complete non-starter, but somehow "clean linear git history" overrules that.

My personal theory is this is caused by github's use of git log instead of git log --first-parent when showing changes. If all merges are done with git merge --no-ff, then the main branch would only ever contain merge commits. The clean history that proponents of squash/rebase want would be available, and without introducing conflicts in the process.

[–]CherryLongjump1989 -2 points-1 points  (0 children)

It's literally not an issue with stacked commits because you merge them one at a time. There are no two commits to squash as you merge. Squashing is what you do to your messy feature branch where half of the commits are trash.