While I'm torn on this as a developer, it comes down to an ease of understanding the code. In all cases, it is easier to understand where something comes from if you only import modules. Enforcing the import of modules tends to also ensure namespace conflicts don't occur as often. When it comes to review, I am going to agree with Sean here, it is a boon on large changes. I am against lessening/removing H302; but I understand why people desire it eased up.<div>
<br></div><div>Cheers,</div><div>Morgan Fainberg</div><div><br></div><div>IRC: morganfainberg<br><div><br></div><div><br><div><div><div><div class="gmail_quote">On Tue, Aug 6, 2013 at 1:18 PM, Christopher Armstrong <span dir="ltr"><<a href="mailto:chris.armstrong@rackspace.com" target="_blank">chris.armstrong@rackspace.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="im">On Tue, Aug 6, 2013 at 6:32 AM, Sean Dague <span dir="ltr"><<a href="mailto:sean@dague.net" target="_blank">sean@dague.net</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div><br></div></div>
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.<br>
<br>
MUST == computer can do it, less work for core review time (which is realistically one of our most constrained resources in OpenStack)<br>
MAY == humans have to make a judgement call, which means more work for our already constrained review teams<br>
<br>
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.<br>
</blockquote><div><br></div><div><br></div></div><div>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.</div>
<div><br></div><div>I recommend having humility in our reviews. Instead of</div><div><br></div><div>"This bike shed needs to be painted red. -1"</div><div><br></div><div>One should say</div><div><br></div><div>
"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"</div>
<div><br></div><div>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.<br></div><div>
<br></div><div>And given the rationale from Robert Collins, I agree that the module-import thing should be one of the flakes that allows exceptions.</div><span class="HOEnZb"><font color="#888888"><div><br></div></font></span></div>
<span class="HOEnZb"><font color="#888888">-- <br><div dir="ltr">IRC: radix<div>Christopher Armstrong</div>
<div>Rackspace</div></div>
</font></span></div></div>
<br>_______________________________________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></blockquote></div><br></div></div></div></div></div>