Re: [boost] [github] default PR destination

Subject: Re: [boost] [github] default PR destination
From: Bjørn Roald (bjorn_at_[hidden])
Date: 2014-01-08 03:38:50

On 01/08/2014 03:27 AM, Cox, Michael wrote:
> On Sun, Jan 5, 2014 at 8:44 AM, Peter A. Bigot <pab_at_[hidden]> wrote:
>
>> On 01/05/2014 09:34 AM, Peter A. Bigot wrote:
>>
>>> On 01/05/2014 08:50 AM, Peter Dimov wrote:
>>>
>>>> Peter A. Bigot wrote:
>>>>
>>>>> The last one (regex) I did off master, and I expect that most future
>>>>> ones will also be off master.
>>>>>
>>>>
>>>> What is a developer supposed to do with a pull request against master?
>>>>
>>>
>>> Pull the branch to a local repository, merge it into develop, test it,
>>> and reject or approve it.  Normal git practice: see
>>> https://help.github.com/articles/merging-a-pull-request under "Merging
>>> locally".
>>>
>>
>> If master and develop do not share commits (as I believe they will not
>> under git-flow):
>
>
> I noticed that there are some submodules where develop is based on master,
> i.e. if you run "git branch --contains master", both master and develop are
> output.  Most of them, like you say, show master and develop to be
> distinct.  Won't that be a problem when trying to merge from develop to
> master?

I am not sure how much it matters, if any what branches is listed by 
"git branch --contains master".

Apply/commit, merge from other remote tracking branches, or pull 
contributions such as GitHub PRs to develop as they where applied to 
trunk in SVN.  It make a lot of sense for all maintainers to maintain 
public devlop head as the point they expect every contributor to base 
their contribution upon.  Every other long running or deferred work not 
expected to go into next release should be put into feature branches.  I 
am not familiar with pulling GitHub PR, but I expect it to be possible 
to merge localy and commit, then before pushing to GitHub make the 
assessment whether it should be put in a feature branch or stay in 
develop, if it need to go in a feature branch you may make the feature 
branch at the new commit and reset develop head to previous commit.

git checkout -b feature/PR-id-descriprion
git checkout develop
git reset HEAD^

this is very simple to do, understand, and verify correctness of what 
you do using some GUIs, e.g. using "gitk" tool that come with git 
distribution. In gitk right click the commits description to select 
"create new branch here" and "reset develop to this" commands.  You see 
the result immediately in the commit tree and know it is what you want 
before pushing to GitHub.  Note, if you want gitk to show more than 
current branch, use gitk --all, or create/edit view in menu.  Don't 
underestimate gitk based on looks, it is actually very elegant and 
powerful to use if you try it a bit.

It is up to the maintainer to merge submodule releases only into master. 
Thus it make no sense in general to base contributions on master as 
develop may contain a lot of stuff for next release.  I would vote 
strongly for always using develop head as base for contributions.

> Just for grins, I took the accumulator submodule in which master and
> develop don't share commits and tried to locally rebase develop onto
> master.  Unfortunately, the rebase failed early, but the few files I looked
> at have the same change trying to be applied on both sides, i.e. the lines
> between >>>> and ==== were the same as the lines between ==== and <<<<<.
>   Any idea why that's the case?

no initial merge in git has been performed yet for accumulators:

https://svn.boost.org/trac/boost/wiki/PostCvtMergePoint

so I don't really know what to expect.  And I don't know if we should 
care much as it is not fair to git to expect much specific when trying 
to blindly merge to disjoint trees.

> It looks like develop might be able to be "reconnected" to master (at least
> in accumulator), but it would take someone who understands the code and
> history of changes.

yes, this needs to be done with some special care as you indicate to 
help Git perform sensible automatic of merge later.

-- 
Bjørn