Handling merging pull requests (PR's)

[Greg Ercolano]

So I have a question about how to handle PRs. I keep running into this and I just don't get it:

    Someone makes an issue
    You create a branch on your fork to solve
    You create a PR
    Some more work is done during the PR to further craft the solution
    Solution is reached, but everyone's tired -- will do the merge later
    Weeks, maybe months pass because of.. whatever

So now FLTK's master has had dozens of commits, and your branch is based on some
now far away commit, and you remember you never merged your work.

Simply, what steps do you do at this point to complete the merge of the pull request?

[Greg Ercolano]

    Just to be clear what I'm looking for, here's the 'before' in my local fork:

O most recent commit (origin/master)
O new-commit
O new-commit
O new-commit
O new-commit
O new-commit
O new-commit
O new-commit
|              O my branch commit 2
|              O my branch commit 1
|              O my branch
O new-commit  /
O new-commit-my-branch-was-based on
O old-commit
O old-commit
..

    ..and here's where I think I want to be after resolving the PR in the upstream master fltk:

O my merge commit (origin/master)
|        \
|          O my branch commit 2
|          O my branch commit 1
|          O my branch
|        /
O most recent commit
O new-commit
O new-commit
O new-commit
O new-commit
O new-commit
O new-commit
O new-commit
O commit-my-branch-was-based on
O old-commit
O old-commit
..

    Is this perhaps a one button thing in github using "Rebase and merge"? i.e.

[imm]

Hi Greg

I don't know GitHub's buttonology for this, but what you showed in green would be a rebate then merge, so that does seem likely.

--
Ian
From my Fairphone FP3
   

--
You received this message because you are subscribed to the Google Groups "fltk.coredev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkcoredev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/fltkcoredev/9139b8f8-d044-b7d0-402e-c95df5c32105%40seriss.com.

[imm]

Phone / typo...

S/rebate/rebase/

[Greg Ercolano]

    Is this perhaps a one button thing in github using "Rebase and merge"? i.e.

On 2020-10-22 00:41, imm wrote:

I don't know GitHub's buttonology for this, but what you showed in green would be a rebate then merge, so that does seem likely.

    I think the one button rebase/merge would flatten out the merge into a linear history.

    I'd kinda prefer to keep the merge a little "knuckle" in the tree, like what gitk shows for commit 7abc09ad8 (**) e.g.

   

    I think with the Rebase/Merge button, it would flatten that out, not sure.

    I'm even having trouble getting that 'knuckle' just experimenting with a local fork; if I rebase my branch and then merge, I end up with a linear looking:

        o merge my branch
	o my branch commit 2
	o my branch commit 1
	o master commit
	o old commit
	o old commit
	..

    ..when what I was hoping to see is:

        o merged my branch
        |\
	| o my branch commit 2
	| o my branch commit 1
        |/
	o master commit
	o old commit
	o old commit
	..

    ..Not sure I understand what it takes to do that.

** The above successful example is actually my own commit, but I had a lot of help from Albrecht to do it,
    and I'm having trouble replicating. The process was a bit convoluted due to various missteps on my part,
    such that I don't have a clear stream of commands to do it again without the missteps, causing me to recently
    botch a merge today that I had to undo, (as apparently there's no way to undo a github merge in a PR;
    once done it's locked and can't be undone/reopened *sigh* which makes the process kinda scary)

[Albrecht Schlosser]

On 10/22/20 1:30 PM Greg Ercolano wrote:
>     Is this perhaps a one button thing in github using "*Rebase and
> merge*"? i.e.

Short answer: yes.

Long answer: it depends ... see below.

> On 2020-10-22 00:41, imm wrote:
>> I don't know GitHub's buttonology for this, but what you showed in
>> green would be a rebate then merge, so that does seem likely.
>

>     I /think/ the one button rebase/merge would flatten out the merge

> into a linear history.
>
>     I'd kinda prefer to keep the merge a little "knuckle" in the tree,
> like what gitk shows for commit 7abc09ad8 (**) e.g.
>

>     I _/think/_ with the Rebase/Merge button, it would flatten that
> out, not sure.

As you noticed there are three options to select from before you press
that GitHub "Merge pull request" button. I'll try to explain the options
more detailed below.

First of all: I suggest to do the *final* action (whatever you want to
do exactly) with the GitHub button because it will record the PR as
"merged" which you can't do otherwise (AFAICT). But before you do this
there are other things you can do in your local repo to fine tune the
result.

>     I'm even having trouble getting that 'knuckle' just experimenting
> with a local fork; if I rebase my branch and then merge, I end up with a
> linear looking:

This 'knuckle' is the result of a "real" merge. This happens
"automatically" if you use `git merge' with a branch that is not
currently based on the tip of the branch you're merging into, i.e. when
your working branch (the PR branch) is based on an older commit of
master and you just merge your branch into/onto master, like this:

$ git checkout master
$ git merge feature-branch

However, if you rebased your feature branch before doing a `git merge'
then there's no need to create that 'knuckle' and git automatically does
a 'fast forward' merge, i.e. it "forwards" the master branch to the tip
of your added branch with one or more commits. This is the default and
obviously not what you want to do here.

> o merge my branch o my branch commit 2 o my branch commit 1
> o master commit
> o old commit
> o old commit
> ..
>
>     ..when what I was hoping to see is:
>
> o merged my branch |\ | o my branch commit 2 | o my branch commit 1 |/ o master commit
> o old commit
> o old commit
> ..
>
>     ..Not sure I understand what it takes to do that.

I think that's basically what we did when we created the rebase and
merge commit you referenced in your message.

> ** The above successful example is actually my own commit, but I had a
> lot of help from Albrecht to do it,
>     and I'm having trouble replicating.

As I wrote above, if you first rebase your branch and then `git merge'
git does a 'fast forward merge' which puts your commits onto the master
branch in a linear sequence. The option to do what you want is to
disallow this behavior with commands like this:

$ git checkout feature-branch
$ git rebase master
$ git checkout master
$ git merge --no-ff feature-branch

The last command (with '--no-ff') adds only one commit (the merge
commit) to the master branch and keeps all commits in the separate
'feature-branch' which creates this 'knuckle' you want. See `git help
merge` or `man git-merge` and scroll down to the description of the
three options "--ff, --no-ff, --ff-only". Citation:

"With --no-ff, create a merge commit [...] even when the merge could
instead be resolved as a fast-forward."

That said, GitHub can do this for you in parts, but if you want to have
finer control over the result you may want to prepare it in your local
repo by using `git rebase -i` so you can edit the commit messages,
squash some commits, or even reorder or delete commits.

Everything you do locally should (and must eventually) be followed by a
`git push` command to push your local 'feature-branch' (the PR branch)
to GitHub before you apply the GitHub "Merge pull request" button. If
you rebased your branch in any way this needs to be a "forced push",
i.e. `git push -f` or `--force`.

Summary: if you committed many single commits and want to rearrange the
results by squashing (some) commits, use `git rebase -i` first. If you
just want to rebase your branch on current master, use `git rebase`
instead. Both should be followed by `git push -f` to push your (local)
results to your feature (PR) branch.

Note that I personally suggest to do this because this is the best
(only) way you can visualize the result (for instance with `git log` or
`gitk`) and *test* (compile, build, test) it before committing/pushing
to the master branch in our FLTK repo.

Finally you should go to GitHub and select one of the three options
which are described in short sentences when you expand the drop down
list. To understand what these options do it's IMHO best to read the
sentence that describes what the particular option will do with the
commits provided in the PR branch. I refer to the image you posted in
this thread before:


(1) "Create a merge commit. *All* commits from this branch will be added
to the base branch via a merge commit."

This means: you will get *one* merge commit and the existing commits in
the merged (PR) branch are retained (the 'knuckle'). Note that this
works also if the PR branch was not based on the top-most commit of the
master branch. The latter will create a longer sequence of commits
"parallel" to the master branch.

(2) "Squash and merge. The 2 (n) commits from this branch will be
combined into one commit in the base branch."

The description is IMHO pretty clear. You will lose the individual
commits (if there are more than one) in the merged (PR) branch, they
will be squashed into one commit and put on top of the master (base)
branch. This creates a linear sequence on master (one additional
commit), no 'knuckle'.

(3) "Rebase and merge. The 2 (n) commits from this branch will be
rebased and added to the base branch."

Again, no 'knuckle'. i.e. no "real" merge commit. The individual commits
of the PR branch "will be rebased and added to the base branch". This
means that the individual commits of the PR branch will be retained but
rebased ("transplanted") onto the tip of the master branch and the
master branch will be "fast forwarded" to include the new commits. This
creates a perfect linear order of the PR branch on top the the current
master branch.


Sorry if this description was longer than necessary, but I hope it was
helpful.

[Greg Ercolano]

On 2020-10-22 05:51, Albrecht Schlosser wrote:

I'm even having trouble getting that 'knuckle' just experimenting 
with a local fork; if I rebase my branch and then merge, I end up with a 
linear looking:

This 'knuckle' is the result of a "real" merge. This happens 
"automatically" if you use `git merge' with a branch that is not 
currently based on the tip of the branch you're merging into, i.e. when 
your working branch (the PR branch) is based on an older commit of 
master and you just merge your branch into/onto master, like this:

$ git checkout master
$ git merge feature-branch

However, if you rebased your feature branch before doing a `git merge' 
then there's no need to create that 'knuckle' and git automatically does 
a 'fast forward' merge, i.e. it "forwards" the master branch to the tip 
of your added branch with one or more commits. This is the default and 
obviously not what you want to do here.

    Ah, OK, so I think the "--no-ff" is the secret sauce I was missing.
    So when I do that to my merge command, I do indeed get the 'knuckle' for my branch -- marvelous!

    Here's the proof on my local fork using 'gitk --all', and doing a File -> Reload after each step, my branch's commits
    shown by the big curly brace:

    I'll read through the rest of your comments too, but that "--no-ff" was the big eureka for that issue.