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

Julian Taylor jtaylor.debian at googlemail.com
Sat Jul 19 07:26:10 EDT 2014


On 19.07.2014 13:04, Ralf Gommers wrote:
> 
> 
> 
> On Sat, Jul 19, 2014 at 12:29 PM, Pauli Virtanen <pav at iki.fi
> <mailto:pav at iki.fi>> wrote:
> 
>     19.07.2014 11:04, Ralf Gommers kirjoitti:
>     [clip]
>     >   1. bugfix PR sent to master by contributor
>     >   2. maintainer decides it's backportable, so after review he
>     doesn't merge
>     > PR but rebases it and sends a second PR. First one, with review
>     content, is
>     > closed not merged.
>     >   3. merge PR into maintenance branch.
>     >   4. send third PR to merge back or forward port the fix to
>     master, and
>     > merge that.
>     > (or some variation with merge bases which is even more involved)
> 
>     The maintainer can just rebase on merge base, and then merge and push it
>     via git as usual, without having to deal with Github. 
> 
> 
> I agree, but note that that's not what's happening in the numpy repo at
> the moment and that Julian (and maybe Chuck as well?) is explicitly
> against any direct pushes. So the 3x more PRs between what the process
> used to be and what Julian proposes is not unrealistic.
> 

It is what is happening at the numpy repo.
We are never directly pushing unreviewed changes, we always have at
least one PR. We only directly push changes that have been approved to
be applied two more than one branch.
With the method I propose there are not any more PRs. You have the main
PR targeted to master and the bugfix PR targeted to the maintenance
branch, it was the same before except the bugfix PR was a cherry pick
instead of a merge.
When directly pushing the second merge we even cut one PR from the process.
E.g. I pushed Pauls PR #4882 directly to 1.9 without asking him to
create a new PR but as far as git is concerned there is no difference,
it as still two merges.

We could always ask for a new PR for the branch merge to see travis
results before the merge. E.g. #4877 and #4891 same branch two PRs two
merges.
I don't think that should be currently required as master and 1.9 are
almost identical and there is little value in seeing travis results for
the second merge before doing the merge.
But when the branches diverge more the two PRs should probably be
preferred to avoid having broken commits on the branches that make
bisecting harder.



More information about the NumPy-Discussion mailing list