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

2014-04-09 Thread Chris Behrens

On Apr 9, 2014, at 12:50 PM, Dan Smith  wrote:

>>> So I'm a soft -1 on dropping it from hacking.
> 
> Me too.
> 
>> from testtools import matchers
>> ...
>> 
>> Or = matchers.Or
>> LessThan = matchers.LessThan
>> ...
> 
> This is the right way to do it, IMHO, if you have something like
> matchers.Or that needs to be treated like part of the syntax. Otherwise,
> module-only imports massively improves the ability to find where
> something comes from.

+1

My eyes bleed when I open up a python script and find 1 million imports for 
individual functions and classes.

- Chris



___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


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

2014-04-09 Thread Dan Smith
>> So I'm a soft -1 on dropping it from hacking.

Me too.

> from testtools import matchers
> ...
> 
> Or = matchers.Or
> LessThan = matchers.LessThan
> ...

This is the right way to do it, IMHO, if you have something like
matchers.Or that needs to be treated like part of the syntax. Otherwise,
module-only imports massively improves the ability to find where
something comes from.

I also think that machine-enforced style, where appropriate, is very
helpful in keeping our code base readable. Repeated patterns and style
help a lot, and anything that can be easily machine-enforced is a win in
my book.

--Dan

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


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

2014-04-09 Thread Duncan Thomas
On 6 August 2013 21:18, Christopher Armstrong
 wrote:

> 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.

It think the problems here are:
(a) death by a thousand cuts - code that has many tiny stylistic
differences is harder to read, harder to reason about and harder to
get correct
(b) if the existing code is of highest possible quality, it sets the
bar for incoming code. Requiring new code to be significantly better
than the old code just makes new contributors resentful.

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


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

2014-04-09 Thread Duncan Thomas
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  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@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



-- 
Duncan Thomas

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


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

2013-08-07 Thread Joe Gordon
On Tue, Aug 6, 2013 at 1:44 PM, Morgan Fainberg  wrote:

> 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.


I agree with Robert, that this can be annoying, but as Sean so eloquently
said one of the main goals of hacking is to reduce the burden on reviews
and to prevent two reviewers from telling the person opposite things.  The
constrained resource in development today is reviewers not developers, so I
am inclined to go with what makes reviewing easier.

In some ways an insane and annoying style requirement, is better then none
as long as it makes code more uniform and makes reviewing easier.

So I too am a soft -1 on dropping this, especially because individual
projects can ignore this per line or per project.




>
> Cheers,
> Morgan Fainberg
>
> IRC: morganfainberg
>
>
> On Tue, Aug 6, 2013 at 1:18 PM, Christopher Armstrong <
> chris.armstr...@rackspace.com> wrote:
>
>> On Tue, Aug 6, 2013 at 6:32 AM, Sean Dague  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.
>>
>
It already is, all imports support the #noqa comment which tells flake8 to
ignore the line.


>
>> --
>> IRC: radix
>> Christopher Armstrong
>> Rackspace
>>
>> ___
>> OpenStack-dev mailing list
>> OpenStack-dev@lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
>
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


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

2013-08-06 Thread Morgan Fainberg
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.

Cheers,
Morgan Fainberg

IRC: morganfainberg


On Tue, Aug 6, 2013 at 1:18 PM, Christopher Armstrong <
chris.armstr...@rackspace.com> wrote:

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


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

2013-08-06 Thread Christopher Armstrong
On Tue, Aug 6, 2013 at 6:32 AM, Sean Dague  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
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


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

2013-08-06 Thread Alex Meade
another -1 from me for dropping it from hacking.

I've been bitten by both lack of explicit usage and review bikeshedding on this 
exact thing.

-Alex

-Original Message-
From: "Sean Dague" 
Sent: Tuesday, August 6, 2013 7:32am
To: openstack-dev@lists.openstack.org
Subject: Re: [openstack-dev] Dropping or weakening the 'only import modules' 
style guideline - H302

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@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


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

2013-08-06 Thread Jay Pipes

On 08/06/2013 07:32 AM, Sean Dague 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.


+1 from me (or another -1 for dropping it from hacking). Sean's points 
above about reviewer time and reducing the back and forth bickering on 
bikeshedding issues is why the rule enforcement is a good idea.


Plus, if someone really wanted to set Or and LessThan into the modules 
local scope, you could just do:


from testtools import matchers
...

Or = matchers.Or
LessThan = matchers.LessThan
...

That way you get both clarity/explicitness as well as convenient 
shortened names.


-jay


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


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

2013-08-06 Thread Sean Dague

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@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


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

2013-08-06 Thread Mate Lakat
Hi,

What about saying that you are not restricted to use H302 in test code
test_*.py ?

On Mon, Aug 05, 2013 at 08:03:26PM -0700, Clint Byrum wrote:
> Excerpts from Robert Collins's message of 2013-08-05 19:26:20 -0700:
> > 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.
> 
> 
> You've convinced me. Monty's point about the complexity of making sure
> what is imported in code is actually a module makes it expensive without
> much benefit.
> 
> I say remove it entirely.
> 
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

-- 
Mate Lakat

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


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

2013-08-05 Thread Clint Byrum
Excerpts from Robert Collins's message of 2013-08-05 19:26:20 -0700:
> 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.


You've convinced me. Monty's point about the complexity of making sure
what is imported in code is actually a module makes it expensive without
much benefit.

I say remove it entirely.

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


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

2013-08-05 Thread Monty Taylor


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.

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


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

2013-08-05 Thread Alex Gaynor
I'd favor weakening or removing this requirement. Besides google I've never
seen any other python project which enforced this standard, and I think
it's a very weak heuristic for readability.

Alex


On Mon, Aug 5, 2013 at 7: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.
>
> -Rob
>
> --
> Robert Collins 
> Distinguished Technologist
> HP Converged Cloud
>
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>



-- 
"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


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

2013-08-05 Thread Robert Collins
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.

-Rob

-- 
Robert Collins 
Distinguished Technologist
HP Converged Cloud

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev