Open Stack

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.

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.

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.

Namespace collision is something I can understand but that can be
alleviated with aliases “import … as …”.

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.

On Wed, Feb 25, 2015 at 12:51 PM, Duncan Thomas <duncan.thomas at gmail.com>
wrote:

> Clint
>
> 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.
>
> 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.
>
> 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.
>
> On 25 February 2015 at 22:46, Clint Byrum <clint at fewbar.com> wrote:
>
>> Excerpts from Duncan Thomas's message of 2015-02-25 10:51:00 -0800:
>> > Hi
>> >
>> > So a review [1] was recently submitted to cinder to fix up all of the
>> H302
>> > violations, and turn on the automated check for them. This is certainly
>> a
>> > reasonable suggestion given the number of manual reviews that -1 for
>> this
>> > issue, however I'm far from convinced it actually makes the code more
>> > readable,
>> >
>> > Is there anybody who'd like to step forward in defence of this rule and
>> > explain why it is an improvement? I don't discount for a moment the
>> > possibility I'm missing something, and welcome the education in that
>> case
>>
>> I think we've had this conclusion a few times before, but let me
>> resurrect it:
>>
>> The reason we have hacking and flake8 and pep8 and etc. etc. is so that
>> code reviews don't descend into nit picking and style spraying.
>>
>> I'd personally have a private conversation with anyone who mentioned
>> this, or any other rule that is in hacking/etc., in a review. I want to
>> know why people think it is a good idea to bombard users with rules that
>> are already called out explicitly in automation.
>>
>> Let the robots do their job, and they will let you do yours (until the
>> singularity, at which point your job will be hiding from the robots).
>>
>> __________________________________________________________________________
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe:
>> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>
>
>
> --
> Duncan Thomas
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150225/824705f2/attachment.html>