[openstack-dev] Bad review patterns

Pedro Roque Marques pedro.r.marques at gmail.com
Thu Nov 7 17:37:35 UTC 2013


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.

btw: this is not at all exclusive to OpenStack. The issues you have pointed out exist for instance in code reviews inside company's proprietary code bases when different teams are involved. I'm yet to see a perfect solution to the problem.

  Pedro.

On Nov 7, 2013, at 3:02 AM, Radomir Dopieralski <openstack at sheep.art.pl> wrote:

> On 06/11/13 09:34, Radomir Dopieralski wrote:
>> Hello,
>> 
>> I'm quite new in the OpenStack project, but I love it already. What is
>> especially nifty is the automated review system -- I'm really impressed.
>> I'm coming from a project in which we also did reviews of every change
>> -- although it was mostly manual, and just one review was enough to
>> merge -- and at some point in that project I noticed that it is very
>> easy to give reviews that are unhelpful, frustrating and just get in the
>> way of the actual work. I started paying attention to how I am reviewing
>> code, and I managed to come up with several patterns that are bad. Once
>> I know the patterns, it's easier to recognize when I'm doing something
>> wrong and rethink the review. I would like to share the patterns that I
>> noticed.
> 
> I created a page on the wiki,
> https://wiki.openstack.org/wiki/CodeReviewGuidelines
> 
> I put some initial content there, based on the discussion in this
> thread. Please feel free to discuss those points further here, and to
> amend that page and add to it.
> 
> Any ideas of where we could put a link to it? I'm thinking about the
> Gerrit Workflow page, maybe also some pages specific to particular teams
> (I've seen there is a page with review tips for Nova).
> 
> -- 
> Radomir Dopieralski
> 
> 
> 
> 
> _______________________________________________
> 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