Re: [openstack-dev] [trove] Request for transparent Gerrit approval process
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 ReviewDayhttps://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
[openstack-dev] [trove] Request for transparent Gerrit approval process
As a non-core, I would like to understand the process by which core prioritizes Gerrit changes. 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? Specifically: * Is a prioritized queue used such as the one found at http://status.openstack.org/reviews/? (This output comes from a project called ReviewDayhttps://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? * 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? * 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. * 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? * 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)? Any clarification of the process would be greatly appreciated. Thanks, Mat ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [trove] Request for transparent Gerrit approval process
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 ReviewDayhttps://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@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
