[openstack-dev] Dropping or weakening the 'only import modules' style guideline - H302

Duncan Thomas duncan.thomas at gmail.com
Wed Apr 9 19:41:37 UTC 2014


I totally agree with Sean. If you're going to weaken the rule in a
codeable way (e.g. it doesn't apply to tests, or to certain named
modules or whatever), then great, fix up the HACKING tool and make the
code slightly more readable. But the general advantages of having the
check outway the costs... -1 on removing it / weakening it unless you
can update the tool to understand the new rule.

On 6 August 2013 12:32, Sean Dague <sean at dague.net> wrote:
> On 08/05/2013 10:38 PM, Monty Taylor wrote:
>>
>>
>>
>> On 08/05/2013 11:26 PM, Robert Collins wrote:
>>>
>>> I wanted to get a temperature reading from everyone on this style
>>> guideline.
>>>
>>> My view on it is that it's a useful heuristic but shouldn't be a
>>> golden rule applied everywhere. Things like matches are designed to be
>>> used as a dsl:
>>>      self.assertThat(foo, Or(Equals("1"), Equals("2")))
>>>
>>> rather than what H302 enforces:
>>>      self.assertThat(foo, matchers.Or(matchers.Equals("1"),
>>> matchers.Equals("2")))
>>>
>>> Further, conflicting module names become harder to manage, when one
>>> could import just the thing.
>>>
>>> Some arguments for requiring imports of modules:
>>>   - makes the source of symbols obvious
>>>     - Actually, it has no impact on that as the import is still present
>>> and clear in the file. import * would obfuscate things, but I'm not
>>> arguing for that.
>>>     - and package/module names can (and are!) still ambiguous. Like
>>> 'test.' - whats that? -> consult the imports.
>>>   - makes mocking more reliable
>>>     - This is arguably the case, but it's a mirage: it isn't a complete
>>> solution because modules still need to be mocked at every place they
>>> are dereferenced : only import modules helps to the extent that one
>>> never mocks modules. Either way this failure mode of mocking is
>>> usually very obvious IME : but keeping the rule as a recommendation,
>>> *particularly* when crossing layers to static resources is a good
>>> idea.
>>>   - It's in the Google Python style guide
>>>
>>> (http://google-styleguide.googlecode.com/svn/trunk/pyguide.html?showone=Imports#Imports)
>>>     - shrug :)
>>>
>>> What I'd like us to do is weaken it from a MUST to a MAY, unless noone
>>> cares about it at all, in which case lets just turn it off entirely.
>>
>>
>> Enforcing it is hard. The code that does it has to import and then make
>> guesses on failures.
>>
>> Also - I agree with Robert on this. I _like_ writing my code to not
>> import bazillions of things... but I think the hard and fast rule makes
>> things crappy at times.
>
>
> 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.
>
> Honestly, after spending the year with the constraint in OpenStack, I'm
> never going to import modules directly in my personal projects, as I think
> the benefits of the explicitness have shown themselves pretty well.
>
> So I'm a soft -1 on dropping it from hacking.
>
>         -Sean
>
> --
> Sean Dague
> http://dague.net
>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



-- 
Duncan Thomas



More information about the OpenStack-dev mailing list