[openstack-dev] Bad review patterns

Mark McLoughlin markmc at redhat.com
Mon Nov 11 22:42:59 UTC 2013


On Thu, 2013-11-07 at 20:40 -0500, David Ripton wrote:
> On 11/07/2013 07:54 PM, Sean Dague wrote:
> > On 11/08/2013 01:37 AM, Pedro Roque Marques wrote:
> >> Radomir,
> >> An extra issue that i don't believe you've covered so far is about comment ownership. I've just read an email on the list that follows a pattern that i've heard many complaints about:
> >> 	-1 with a reasonable comment, submitter addresses the comment, reviewer never comes back.
> >>
> >> Reviewers do need to allocate time to come back and follow up on the answers to their comments.
> >>
> >> Perhaps there is an issue with the incentive system. You can earn karma by doing a code review... certainly you want to incentivise developers that help the project by improving the code quality. But if the incentive system allows for "drive by shooting" code reviews that can be a problem.
> >
> > It's not really an incentive system problem, this is some place where
> > there are some gerrit limitations (especially when your list of reviewed
> > code is long). Hopefully once we get a gerrit upgrade we can dashboard
> > out some new items like that via the new rest API.
> >
> > I agree that reviewers could be doing better. But definitely also
> > realize that part of this is just that there is *so* much code to review.
> >
> > Realize that most core reviewers aren't ignoring or failing to come back
> > on patches intentionally. There is just *so* much of it. I feel guilty
> > all the time by how big a review queue I have, but I also need a few
> > hours a day not doing OpenStack (incredible to believe). This is where
> > non core reviewers can really help in addressing the first couple of
> > rounds of review to prune and improve the easy stuff.
> >
> > We're all in this together,
> 
> Is there a way for Gerrit to only send email when action is required, 
> rather than on any change to any review you've touched?  If Gerrit sent 
> less mail, it would be easier to treat its mails as a critical call to 
> action to re-review.  (There's probably a way to use fancy message 
> filtering to accomplish this, but that would only work for people 
> willing/able to set up such filtering.)

I know you're discounting filtering here, but FWIW I filter on the email
body containing:

  Gerrit-Reviewer: Mark McLoughlin

so that I have all email related to reviews I'm subscribed to in a
single folder. I try hard to stay on top of this folder to avoid being a
drive-by-reviewer.

I group the mails by thread, so I don't mind all the email gerrit sends
out but if e.g. I wanted to only see notifications of new patch sets I'd
filter on:

  Gerrit-MessageType: newpatchset

Mark.




More information about the OpenStack-dev mailing list