search

drewdeponte / git-ps-rs

Official git-ps Rust implementation - the future of git-ps
https://git-ps.sh
MIT License
78 stars 8 forks source link

Merge commits don't get cherry picked properly #260

Closed drewdeponte closed 10 months ago

drewdeponte commented 10 months ago

When you do a gps rebase which simply does an interactive rebase using actual git it seems to look at the two parents of the merge commit and instead of copying the merge commit, it instead cherry picks the commits between that merge commit and that parents leg of the down to the common ancestor, on top of the rest of the commits from parent 0s leg.

Right now when we do any command that uses our custom cherry picking it actually coppies the merge commit over, but it does so in a way that drops the parent 1 leg of commits. So you end up with a merge commit that isn't actually merging anything.

* 255d2cc - G - (origin/ps/rr/merge_branch__foo_, ps/rr/merge_branch__foo_) Merge branch 'foo' (14 minutes ago) <Drew De Ponte>
| * ef25fe1 - G - (HEAD -> main) Merge branch 'foo' (14 minutes ago) <Drew De Ponte>
| * 8ed1ae1 - G - Add comment to the end (18 hours ago) <Drew De Ponte>
| * 6790cb5 - G - Add comment to the end (18 hours ago) <Drew De Ponte>
| * 595de71 - G - There is no world here (2 days ago) <Drew De Ponte>
| * bac8370 - G - Make the world nice (2 days ago) <Drew De Ponte>
| * 8583268 - G - Say hello not goodbye (2 days ago) <Drew De Ponte>
| * 512eafb - G - Make it Zar (2 days ago) <Drew De Ponte>
| * e116459 - G - Make it Car (2 days ago) <Drew De Ponte>
| * 3994e66 - G - Make it Bar (2 days ago) <Drew De Ponte>
| * 0f24478 - G - Make it a crule world (2 days ago) <Drew De Ponte>
|/
| * f89bb3a - G - (foo) Add comment to the end (18 hours ago) <Drew De Ponte>
|/
| * 839bb44 - G - (far) Make it Zar (2 days ago) <Drew De Ponte>
| * 8452e63 - G - Make it Car (2 days ago) <Drew De Ponte>
| * 7a2f4da - G - Make it Bar (2 days ago) <Drew De Ponte>
|/
* 7ac91bb - G - (origin/main, origin/HEAD) Change to Cindy (2 days ago) <Drew De Ponte>

This should be addressed probably by doing what the interactive rebase dose.

drewdeponte commented 10 months ago

Ok, I thought about this a bit more this weekend. I think the right thing to do is to actually check for merge commits and bail out of the command with an error. Letting the user know that a merge commit was detected and that they need to first do a gps rebase on their stack to flatten any merge commits.

The reason I think this makes sense is because the stack is where you iterate on patches to get things into a good state prior to requesting review or integrating them. Having a merge commit in the stack goes against the linear history idea that the stack is founded around. Therefore, I think we should take the stance that users should have to git rid of (flatten out) any merge commits in the stack so that they can create linear history patch series when requesting review or integrating. And the easiest way to do that is to simply run gps rebase as it will flatten merge commits.

drewdeponte commented 10 months ago

This has been integrated into mainline. Therefore I am closing this issue.