<div dir="ltr">The “making things more explicit” argument carried to its logical conclusion would require that no “import … as …” ever be used either, and we would have to use the full namespace/path for each module or function being used. With allowing “import … as …” and the practice of extending and monkey patching modules, there is enough obfuscation that I sometimes can’t tell where things come from despite having this rule in place. So I’m not sure that’s the best argument for this rule. I’m also not saying “import … as …” should not be allowed either.<div><br></div><div>Also, having to adhere to this particular rule has meant that I end up with some lines of code that run afoul of the 79 character per line rule when code is nested in more than one or two blocks and I end up having to break up lines of code in weird ways to satisfy these two rules, which I don’t think helps with “understandability” and “legibility” of code very much.</div><div><br></div><div>To the above point I’m sure someone would say “refactor the code”, and I’m not sure creating functions with just a couple lines for the purpose of meeting these rules is necessarily the best thing either.</div><div><br></div><div>Namespace collision is something I can understand but that can be alleviated with aliases “import … as …”.</div><div><br></div><div>The point wrt to mock is an interesting one, and seems to be the best reason for keeping the rule in place if true, but personally wish it wasn’t so.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 25, 2015 at 12:51 PM, Duncan Thomas <span dir="ltr"><<a href="mailto:duncan.thomas@gmail.com" target="_blank">duncan.thomas@gmail.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><div><div>Clint<br><br></div>This rule is not currently enabled in Cinder. This review fixes up all cases and enables it, which is absolutely 100% the right thing to do if we decide to implement this rule.<br><br></div>The purpose of this thread is to understand the value of the rule. We should either enforce it, or else explicitly decide to ignore it, and educate reviewers who manually comment on it.<br><br></div>I lean against the rule, but there are certainly enough comments coming in that I'll look and think again, which is a good result for the thread.<br></div><div class="gmail_extra"><div><div class="h5"><br><div class="gmail_quote">On 25 February 2015 at 22:46, Clint Byrum <span dir="ltr"><<a href="mailto:clint@fewbar.com" target="_blank">clint@fewbar.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Excerpts from Duncan Thomas's message of 2015-02-25 10:51:00 -0800:<br>
<span>> Hi<br>
><br>
> So a review [1] was recently submitted to cinder to fix up all of the H302<br>
> violations, and turn on the automated check for them. This is certainly a<br>
> reasonable suggestion given the number of manual reviews that -1 for this<br>
> issue, however I'm far from convinced it actually makes the code more<br>
> readable,<br>
><br>
> Is there anybody who'd like to step forward in defence of this rule and<br>
> explain why it is an improvement? I don't discount for a moment the<br>
> possibility I'm missing something, and welcome the education in that case<br>
<br>
</span>I think we've had this conclusion a few times before, but let me<br>
resurrect it:<br>
<br>
The reason we have hacking and flake8 and pep8 and etc. etc. is so that<br>
code reviews don't descend into nit picking and style spraying.<br>
<br>
I'd personally have a private conversation with anyone who mentioned<br>
this, or any other rule that is in hacking/etc., in a review. I want to<br>
know why people think it is a good idea to bombard users with rules that<br>
are already called out explicitly in automation.<br>
<br>
Let the robots do their job, and they will let you do yours (until the<br>
singularity, at which point your job will be hiding from the robots).<br>
<div><div><br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</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>
</div></div></blockquote></div><br><br clear="all"><br></div></div><span class="HOEnZb"><font color="#888888">-- <br><div>Duncan Thomas</div>
</font></span></div>
<br>__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</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>