openstack team mailing list archive

[Mark McLoughlin <markmc@xxxxxxxxxx>]

Hey,

On Wed, 2011-11-09 at 16:50 +0100, Thierry Carrez wrote:
> Hi everyone,
> 
> Since there seems to be some confusion around master vs. stable/diablo
> vs. core reviewers, I think it warrants a small thread.
> 
> When at the Design Summit we discussed setting up stable branches, I
> warned about the risks that setting them up brings for trunk development:
> 
> 1) Reduce resources affected to trunk development
> 2) Reduce quality of trunk

The "it must be in trunk first" policy is the best way to mitigate
against that.

> To mitigate that, we decided that the group doing stable branch
> maintenance would be a separate group (i.e. *not* core developers), and
> we decided that whatever ends up in the stable branch must first land in
> the master branch.

Well, I recall it a little differently - that both the stable branch
maint group and the core team members would have +2 privileges on the
stable branch.

Maybe I just misread James here:

  https://lists.launchpad.net/openstack/msg04751.html

  "only members of the maintainers team or core devs can +/-2"

I also seem to have imagined the core teams being members of the stable
branch maint team:

  https://launchpad.net/~openstack-stable-maint/+members

But, whatever :)

We have a separate team with me, Chuck and Dave on it. Only one of us
can +2.

But wait! Vish +2ed a stable branch patch yesterday:

  https://review.openstack.org/328

James, help a poor confused soul out here, would you? :)

> So a change goes like this:
> * Change is proposed to trunk
> * Change is reviewed by core (is it appropriate, well-written, etc)
> * Change lands in trunk
> * Change is proposed to stable/diablo
> * Change is reviewed by stable team (is it relevant for a stable update,
> did it land in trunk first)
> * Change lands in stable/diablo
> 
> This avoids the aforementioned risks, avoids duplicating review efforts
> (the two reviews actually check for different things), and keep the
> teams separate (so trunk reviews are not slowed down by stable reviews).
> 
> Note that this does not prevent core developers that have an interest in
> stable/diablo from being in the two teams.
> 
> Apparently people in core can easily mistake master for stable/diablo,
> and can also +2 stable/diablo changes. In order to avoid mistakes, I
> think +2 powers on stable/diablo should be limited to members of the
> stable maintenance team (who know their stable review policy).
> 
> That should help avoid mistakes (like landing a fix in stable/diablo
> that never made it to master), while not preventing individual core devs
> from helping in stable reviews.

Right, that makes sense. Only folks that understand the stable branch
policy[1] should be allowed to +2 on the stable branch.

Basically, a stable branch reviewer should only +2 if:

  - It fixes a significant issue, seen, or potentially seen, by someone
    during real life use

  - The fix, or equivalent, must be in master already

  - The fix was either a fairly trivial cherry-pick that looks 
    equally correct for the stable branch, or that the fix has 
    sufficient technical review (e.g. a +1 from another stable 
    reviewer if it's fairly straightforward, or one or more +1s from 
    folks on core it it's really gnarly)

  - If this reviewer proposed the patch originally, another stable
    branch reviewer should have +1ed it 

All we need is an understanding of the policy and reasonable judgement,
it's not rocket science. I'd encourage folks to apply to the team for
membership after reviewing a few patches.

Cheers,
Mark.

[1] - http://wiki.openstack.org/StableBranch