openstack team mailing list archive

[corvus@xxxxxxxxxxxx (James E. Blair)]

Hi,

Monty and I have been working on a solution to a number of problems
and suggested enhancements to Gerrit.  In short, we're planning on
adding a new review type to Gerrit so that a core reviewer can
specifically mark a review as "Approved" for Jenkins to test and
merge.  This will end our overloading of the "+2" code review for that
purpose.  We're planning on upgrading Jenkins and making these changes
on Tuesday, December 27th.  Read below for our rationale, or skip to
the end to see specifically what workflow will change.

Rationale:

One of the problems we want to solve is that it's difficult to see
which votes are from project-core members (bug 900413).  Even though
with Gerrit ACLs only project-core members can vote code review +2,
because we use that as the trigger for Jenkins jobs and ask core
reviewers to vote +1 until the change is ready to go in, it's not
obvious which votes are from core team members.  By changing the
Jenkins trigger to something else, the full range of values for code
review votes is again available for use.  Any number of core reviewers
can vote +2, and their doing so indicates to anyone looking at the
review that they are a core reviewer.  A person looking to see if a
patch has been sufficiently reviewed for inclusion can look for 2 +2
votes.  A core reviewer who doesn't feel very strongly about his or
her review can even vote +1 to say "I like this but I don't want my
vote to count as a core reviewer vote".

We're upgrading to a new version of the Gerrit Trigger plugin for
Jenkins that allows the selection of different trigger criteria (for
instance, when a patchset is uploaded or when a change is merged), and
to that we're added a configurable trigger for when a vote is left for
a review (bug 903375).  Now that they are available, we will be using
all of the above trigger types in Jenkins to make our job
configuration model closer to what we want.

Now that we have the ability to easily specify what Gerrit action
should trigger a job in Jenkins, we will add a new review category to
Gerrit called "Approved".  Core reviewers (only) will be able to vote
0 or 1 in this category, and when they do so, the change will be
considered approved, and Jenkins will immediately run trunk gating
tests and merge the change on completion.  It will also appear as a
separate column on pages that list reviews (labeled "A", for
"Approved", along with "V" for "Verified", and "R" for "Code Review").
This will make it easy to see which reviews have been approved, and
which may or may not need to be approved.

Proposed Workflow Changes:

* Code review +2 votes will no longer trigger Jenkins.

* Core reviewers may now feel free to vote with any value between -2
  and +2 on any change.  They should vote +2 if they want their vote
  to be counted as meeting the inclusion criteria of "positive reviews
  from at least 2 core developers".

* Core reviewers should periodically scan for reviews that are ready
  for approval and approve them if they feel they have been
  sufficiently reviewed and the inclusion criteria are met.

* To approve a change and start the merge process, core reviewers
  should vote +1 in the "Approved" category (click the review button
  again, and only change the value for the "Approved" category).

* These changes will go into effect on Tuesday, December 27th.

This wiki page has some screenshots of what the above changes will
look like:

  http://wiki.openstack.org/GerritWorkflowChanges

Thanks,

Jim