[Numpy-discussion] proposal: new commit guidelines for backportable bugfixes

[Julian Taylor]

On Fri, Jul 18, 2014 at 5:38 PM, Charles R Harris
<charlesr.harris at gmail.com> wrote:
>
>
> On Fri, Jul 18, 2014 at 8:13 AM, Julian Taylor
> <jtaylor.debian at googlemail.com> wrote:
>>
>> hi,
>> I have been doing a lot of backporting for the last few bugfix
>> releases and noticed that our current approach committing to master
>> and cherrypicking is not so good for the git history.
>> When cherry picking a bugfix from master to a maintenance branch both
>> branches contain a commit with the same content and git knows of no
>> relation between them. This causes unnecessary merge conflicts when
>> cherry picking two changes that modify the same file. The git version
>> (1.9.1) I am using is not smart enough too figure out the changesets
>> in both leaf commits is the same.
>> Additionally the output of `git log maintenance/1.9.x..master` becomes
>> very large as all already backported issues appear again in master.
>> [0]
>>
>> To help with this I want to propose new best practices for pull
>> requests of bugfixes suitable for backporting.
>> Instead of basing the bugfix on the head commit of the master, base
>> them on the merge base between master and the latest maintenance
>> branch.
>> This allows merging the PR into both master and the maintenance branch
>> without pulling in any extra changes from either branches.
>> Then both branches contain the same commit and gits automerging can
>> work better and git log will only show you the commits that are only
>> really on one branch or the other.
>>
>> In practice this is very simple. You can still develop your bugfix on
>> master but before you push it you just run:
>>
>> git rebase --onto $(git merge-base master maintenance/1.9.x) HEAD^
>>
>> In most bugfix PRs this should work without conflict as they should be
>> relatively small.
>> If you get a merge conflict during this operation, just do git rebase
>> --abort and do a normal pull request, in that case the backporter
>> should worry about the conflict.
>>
>> Does this sound like a reasonable procedure?
>> Cheers,
>> Julian
>>
>> [0] git cherry is supposed to help with that, but it never really
>> worked properly for me
>
>
> Arrived here promptly. This looks OK to me, but with the understanding that
> a number of folks won't know what is going on. It should be documented in
> doc/source/dev/gitwash/development_workflow.rst and perhaps a command alias
> in .git/config would help, something like npyrebase, or hopefully something
> better ;)
>
> Chuck
>

Yes of course I would document it when its ok for everyone.
I do not want that this inconveniences contributors, maybe we can just
ask for it if extra changes are required for the PR anyway.
I would just like that the people who merge PR's (which are currently
just a handful) try to use this method when the PR is applicable for a
maintenance branch.
We can add a small tool that does what does the rebase, merges to both
master and the branch and closes the PR, something like:
tools/merge-backport-pr #pr-number <maintenance-branch>