Merge master into fix branch during review

[Kevin Rushforth]

When you *initially* create the PR you can (and are encouraged to) 
rebase your branch on master.

Subsequent updates to bring your branch up-to-date with master should be 
done with a "git merge". The contributing guidelines strongly recommend 
that you do not rebase your branch once you publish it (which, in this 
context, means once you first make a PR using that branch "rfr"). Skara 
now also warns you not to do that.

The main reason is that rebasing an in-progress PR makes it hard for 
people who have been following along as the review progresses to see 
incremental changes or go back and make sure that earlier comments have 
been addressed. I agree that it isn't quite as much of a problem if you 
don't squash or alter commits, but the tooling doesn't always make that 
easy to see.

-- Kevin


On 8/15/2022 1:33 PM, John Hendrikx wrote:
> Normally pulling in changes from master is done by rebasing the branch 
> on master, while putting in changes from a feature branch into master 
> is a merge. This avoids making it seem the commits on master are part 
> of your branch, and even avoids accidentally including something from 
> master when your branch is merged but the change on master was 
> reverted before that happened.
>
> Is there a reason openjfx uses a merge to bring a branch up-to-date 
> with master? As long as the commits on the branch are not squashed or 
> altered it should not make reviewing any harder.
>
> --John
>
> On 15/08/2022 12:53, Jeanette Winzenburg wrote:
>>
>> .. is something I _personally_ don't like: have to mentally sort the 
>> related from the unrelated commits in the history.
>>
>> The contributing guidelines do allow intermediate merges (bolding by me)
>>
>> "If you __need__ to pick up changes from master, you can merge master 
>> into your branch"
>>
>> my interpretation would be: don't without good reason.
>>
>> To merge or not to merge, that is the quesion :)
>>
>> -- Jeanette
>>