search

isaacs / github

Just a place to track issues and feature requests that I have for github
2.2k stars 129 forks source link

Support semi-linear merge option for pull requests #1017

Open waldyrious opened 7 years ago

waldyrious commented 7 years ago

This would be equivalent to rebasing a PR without merging it (tracked as #88, most explicitly described in @jagthedrummer's comment in that thread), and then using the normal merge option (which always creates a merge commit, even if the merge could be fast-forwarded).

Such a workflow can be implemented via the command line, or using third-party tools, as @galulex illustrated here, but it would be best if it was a natively supported additional option in GitHub's web UI.

FYI, GitLab has implemented this about a year ago, and it's a popular request for Bitbucket as well.

To make this more concrete and visual, I found that the diagram from the post A tidy, linear Git history explains the difference between these workflows quite well in a visual way, and I took the liberty to add a pure linear workflow to the diagram to quickly convey the differences between merge vs. rebase && merge --ff-only (linear) vs. rebase && merge --no-ff (semi-linear):

git history diagrams

insearching commented 4 years ago

Any updates here?

mbitsnbites commented 3 years ago

I second this suggestion. Most of my projects follow the stable mainline model that relies on rebase && merge --no-ff (a.k.a. "merge commit with semi-linear history").

image

Right now I have to go through hoops to achieve the same thing in GitHub:

  1. Check if the PR branch is based on the latest commit on master. This is quite cumbersome in GitHub since that information is not available in the PR (to my knowledge), but rather I have to go to the source/fork repo & branch and check the history to find the SHA of the commit that comes before the first PR commit, and compare that to the SHA of the tip of master in the target repo & branch.
  2. If the PR branch is based on the tip of master, great! Just pick the "Create merge commit" option and merge. Done! Obviously this assumes that no commits are pushed to the master branch after the SHA comparison was done. In personal projects with a single maintainer that is usually not a problem. In larger projects this can be a real problem though.
  3. Otherwise I have two options: a. Instruct the PR author to rebase the branch & force push. This can be problematic if there are several concurrent PR:s in progress (and honestly it can be a bit awkward to put this extra work on the contributor). b. Rebase the branch myself, close the old PR, create a new PR and use the "Create merge commit" option.

In any event, this means a lot of manual work that could and should be automated.

Ideally, any CI checks should be re-run with the rebased branch before doing merge --no-ff, but even without that functionality there would be great value in a fourth "Create merge commit with semi-linear history" option.

mbitsnbites commented 3 years ago

The extra option could look something like this: image

K3UL commented 3 years ago

Would that include the fact to preserve commits SHAs ? Github rebasing and changing the SHA is a problem in my case to compare where a branch is ahead of another

waldyrious commented 3 years ago

@K3UL The only way to keep the graph semi-linear (i.e. avoid crossing lines as in the first diagram in the opening comment) would be to rebase PR branches whose base has lagged behind the target branch, and that would necessarily change the commit hashes. But I would definitely be in favor of preserving the branch's existing commit hashes if it doesn't require rebasing (i.e. if a fast-forward merge would be possible).

essenmitsosse commented 3 years ago

@waldyrious there already is a protected branch rule "Require branches to be up to date before merging" that enforces the PR commits to be ahead of the current master — thus already be rebased. Enabling this rule could be a requirement to be able to use merging with linear history.

I think the big advantage here is: the commit being build in the PR is guaranteed to be the one that ends up being the new HEAD of master. Which is great, because there is absolutely nothing changing after the PR is closed. We already enforce this in our repos, but currently have to do this manually and merge the feature-branch by hand on our locale machine.

It might be a bit cumbersome for large repos, with several PRs being open in parallel, but I think for a lot of repos, the super clean history is worth it.

chucklu commented 3 years ago

@waldyrious The problem of fast-forward merge is, you could not easily tell a merge operation from the log graph. That's why we need semi-linear merge. Semi-linear will not re-generate new commits for existing commits, when the commits can be fast-forwarded, it will only generate a new merge commit.

sandstrom commented 3 years ago

If you want this feature, send it to the Github team working on PRs, merge button, etc.

Take 1 minute and send your feedback via their form here: https://support.github.com/contact/feedback?category=prs-and-code-review

(click upvote on this comment if you've emailed them)

waldobronchart commented 3 years ago

For anyone watching this thread, I wrote a script to do this because I was tired of waiting on Github.

Get the script here: https://pypi.org/project/git-pr-linear-merge/

It does what the original post describes: rebase, then merge without fast-forward with a simple terminal command: git pr merge XXX

Documentation can be found on the project page.

black-puppydog commented 3 years ago

@waldobronchart the GH repo linked from PyPI is private, you might wanna change that. :) Also why did you use "linear" for what is called "semi-linear" here?

waldobronchart commented 3 years ago

I gotta fix the source link, the repo is https://github.com/waldobronchart/git-pr-linear-merge

Also why did you use "linear" for what is called "semi-linear" here?

Yeah I realized it after that that was the standard name, but I don't want to rename the package now because it'll cause confusion 🤷