Open Stack

On Tue, Aug 6, 2013 at 6:32 AM, Sean Dague <sean at dague.net> wrote:

>
> The reason we go hard and fast on certain rules is to reduce review time
> by people. If something is up for debate we get bikeshedding in reviews
> where one reviewer tells someone to do it one way, 2 days later they update
> their review, another reviewer comes in and tells them to do it the
> otherway. (This is not theoretical, it happens quite often, if you do a lot
> of reviews you see it all the time.) It also ends up being something
> reviewers can stop caring about, because the machine will pick it up.
> Giving them the ability to focus on higher order issues, and still keeping
> the code from natural entropy.
>
> MUST == computer can do it, less work for core review time (which is
> realistically one of our most constrained resources in OpenStack)
> MAY == humans have to make a judgement call, which means more work for our
> already constrained review teams
>
> I've found H302 to really be useful on reviewing large chunks of code I've
> not been in much before. And get seriously annoyed being in projects that
> don't have it enforced yet (tempest is guilty of that). Being able to
> quickly know what namespace things are out of saves time.
>


I think it's really unfortunate that people will block patches based on
stylistic concerns. The answer, IMO, is to codify in policy that stylistic
issues *cannot* block a patch from landing.

I recommend having humility in our reviews. Instead of

"This bike shed needs to be painted red. -1"

One should say

"I prefer red for the color of bike sheds. You can do that if you want, but
go ahead and merge anyway if you don't want to. +0"

and don't mark a review as -1 if it *only* has bikeshedding in it. I would
love to see a culture of reviewing that emphasizes functional correctness,
politeness, and mutual education.

And given the rationale from Robert Collins, I agree that the module-import
thing should be one of the flakes that allows exceptions.

-- 
IRC: radix
Christopher Armstrong
Rackspace
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20130806/8e3e8003/attachment.html>