Open Stack

Thanks Matt for raising this topic once again, I strongly agree with
you, I've even added the following wording to our backports policy back
in March to address the same problem:

  "For a bug targeted for stable release series, cherry-pick the fix
  commit onto the stable/x.x branch for each series it is targeted for,
  *after* the fix was merged to master (creating backports prematurely
  leads to possible inconsistency between master and stable/x.x versions
  of the same commit)."

[0] https://wiki.openstack.org/w/index.php?title=Fuel%2FHow_to_contribute&diff=75254&oldid=74351

At the same time I agree with Dmitry, we should not abuse the -2 code
review vote, and leave it for indicating that the commit should not be
merged even if updated (in case of backports, indicating that the code
change is too disruptive to be allowed in a stable branch).

I think we can deal with this on a case by case basis.

If you are a core reviewer looking at a commit to a stable branch, and
the master version of the commit is not yet merged, just quote the above
fragment from the policy in your -1 comment (unless other reviewers
already done that) and move on.

If you notice someone repeating this mistake more than once, reach out
to them in private and point them at the wiki and at this thread, and
explain that this kind of behaviour is harmful in that it pollutes
review queue and distracts reviewers.

Thanks,

-- 
Dmitry Borodaenko


On Fri, Oct 02, 2015 at 02:56:50PM +0300, Dmitry Pyzhov wrote:
> I'm not sure if core reviewers should be so harsh. But the guideline seems
> to be very useful. Guys, please don't create backports too early.
> 
> On Fri, Oct 2, 2015 at 2:00 PM, Matthew Mosesohn <mmosesohn at mirantis.com>
> wrote:
> 
> > Hi Fuelers,
> >
> > I would like to address a concern I have with backporting policy. I'm sure
> > all of you know that we should always land patches to master before it
> > reaches stable/X.X branch. What you are not aware of probably is that many
> > people are making cherry picks well in advance of gathering reviews and
> > getting the patch landed in master. Some argue that it "saves time waiting
> > on CI", but in reality it's quite the opposite. Adding a cherry pick before
> > merging master causes the following workflow to take place:
> > 1 - Propose to master and to stable/7.0
> > 2 - CI runs on 2 patches
> > 3 - Reviewer A comments on master patch
> > 4 - owner adjusts both patches and runs CI
> > 5 - Reviewer B comments on stable patch
> > 6 - owner adjusts both patches and runs CI
> > (repeat 3-6 in varying degrees until enough patches are gathered)
> > 7 - rebase stable/7.0 patch again... wait for CI again
> >
> > This doubles the burden on CI and complicates the overall review process
> > where we are accepting feedback for the initial solution on two (nearly)
> > identical patches. What's worse is it's possible that the two solutions
> > merged won't be identical and introduce potential regressions.
> >
> > I propose we avoid raising any stable/X.X patches before a patch is
> > _merged_ into master to avoid this scenario. Additionally, if a core sees
> > that this is happening, he or she should mark it -2 and discourage
> > submission of new patchsets.
> >
> > I welcome your thoughts and feedback.
> >
> > Best Regards,
> > Matthew Mosesohn
> >
> > __________________________________________________________________________
> > OpenStack Development Mailing List (not for usage questions)
> > Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >
> >

> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev