[openstack-dev] [trove] Request for transparent Gerrit approval process

Morgan Jones morgan at parelastic.com
Tue May 6 14:17:44 UTC 2014


I've been thinking about this a bit, and had some ideas.  Take the 
following as points for thought, rather than my saying "this is what we 
should do".

How about each Trove participant be assigned a core mentor.  This way, 
each non-core person knows who they can ask for information about how 
OpenStack and Trove work.

During the blueprint review process, part of "approving" a blueprint 
would be assigning a core member to oversee the implementation of that 
blueprint.  The assigned core member needn't have an active role in the 
implementation of the blueprint, but they would be available to answer 
questions and provide assistance where needed. Likewise, part of 
triaging a bug would be assigning a core member to oversee it.

When a blueprint or bug is complete and ready for review, it would be 
reviewed by the implementer's mentor and the core member who is 
overseeing the bp/bug.  This way, reviews needn't sit around waiting for 
someone to notice them.

Hopefully, such a system would help core members plan their workload.  
It should also make non-core member's integration into the team faster 
and easier, and perhaps  even speed increasing the core team by 
providing consistent mentoring.

Thoughts?

Morgan.


On 05/05/2014 10:29 PM, Nikhil Manchanda wrote:
> Hi Mat:
>
> Some answers, and my perspective, are inline:
>
> Lowery, Mathew writes:
>
>> As a non-core, I would like to understand the process by which core
>> prioritizes Gerrit changes.
> I'm not aware of any standard process used by all core reviewers to
> prioritize reviewing changes in Gerrit. My process, specifically,
> involves looking through trove changes that are in-flight in gerrit, and
> picking ones based on priority of the bug / bp fixed, whether or not the
> patch is still work-in-progress and age.
>
>
>> I'd also like to know any specific
>> criteria used for approval. If such a process was transparent and
>> followed consistently, wouldn't that eliminate the need for "Hey core,
>> can you review <change>?" in IRC?
> I'm not aware of any specific criteria that we use for approval other
> than the more general cross-OpenStack criteria (hacking, PEP8,
> etc). Frankly, any rules that constitute a common criteria should really
> be enforced in the tox runs (for eg. through hacking). Reviewers should
> review patches to primarily ensure that changes make sense in context,
> and are sound design-wise -- something that automated tools can't do (at
> least not yet).
>
>
>> Specifically:
>>
>>    *   Is a prioritized queue used such as the one found at
>>    http://status.openstack.org/reviews/? (This output comes from a
>>    project called
>>    ReviewDay<https://github.com/openstack-infra/reviewday> and it is
>>    prioritized based on the Launchpad ticket and age:
>>    http://git.io/h3QULg.) If not, how do you keep a Gerrit change from
>>    "starving?"
> How reviewers prioritize what they want to review is something that is
> currently left up to the reviewers. Personally, when I review changes,
> I quickly look through all patches on review.o.o and do a quick triage
> to decide on the review order. I do look at review-age as part of the
> triage, and hopefully this prevents 'starvation' to some extent.
>
> That said, this area definitely seems like one where having a
> streamlined, team-wide process can help with getting more bang for our
> review-buck. Using something like reviewday could definitely help here.
>
> I'm also curious how some of the other OpenStack teams solve this issue,
> and what we can do to align in this regard.
>
>
>>    *   Is there a baking period? In other words, is there a minimum
>>    amount of time that has to elapse before even being considered for
>>    approval?
> There is no such baking period.
>
>
>>    *   What effect do -1 code reviews have on the chances of approval
>>    (or even looking at the change)? Sometimes, -1 code reviews seem to
>>    be given in a cavalier manner. In my mind, -1 code reviewers have a
>>    duty to respond to follow-up questions by the author. Changes not
>>    tainted with a -1 simply have a greater chance of getting looked at.
> I always look at the comment that the -1 review has along with it. If
> the concern is valid, and the patch author has not addressed it with a
> reply, I will leave a comment (with a -1 in some cases) myself. If the
> original -1 comment is not applicable / incorrect, I will +1 / +2 as
> applicable, and usually, also leave a comment.
>
>
>>    *   To harp on this again: Isn't "Hey core, can you review
>>    <change>?" inherently unfair assuming that there is a process by
>>    which a Gerrit change would normally be prioritized and reviewed in
>>    an orderly fashion?
> I don't necessarily see this as unfair. When someone asks me to review a
> change, I don't usually do it immediately (unless it's a fix for a
> blocking / breaking change). I tell them that I'll get to it soon, and
> ensure that it's in the list of reviews that I triage and review as
> usual.
>
>
>>    *   Are there specific actions that non-cores can take to assist in
>>    the orderly and timely approval of Gerrit changes? (e.g. don't give
>>    a -1 code review on a multiple +1'ed Gerrit change when it's a
>>    nice-to-have and don't leave a -1 and then vanish)?
> IMHO purely 'nice-to-have' nitpicks should be +0, regardless of how
> many +1s the review already has. And if someone leaves a -1 on a review
> and subsequently refuses to discuss it, that is just bad form on
> the part of the reviewer. Thankfully, this hasn't been a big problem
> that we've had to contend with.
>
>
>> Any clarification of the process would be greatly appreciated.
>>
>> Thanks,
>> Mat
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev




More information about the OpenStack-dev mailing list