On Tue, Aug 6, 2013 at 6:32 AM, Sean Dague <[email protected]> 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
_______________________________________________ OpenStack-dev mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
