On 2014-01-23 04:16, Steven Hardy wrote:
On Thu, Jan 23, 2014 at 07:54:34AM +1000, Angus Salkeld wrote:
On 22/01/14 12:21 +0000, Steven Hardy wrote:
>On Wed, Jan 22, 2014 at 01:23:05PM +0200, Pavlo Shchelokovskyy wrote:
>>Hi all,
>>
>>we have an approved blueprint that concerns reducing number of ignored PEP8
>>and openstack/hacking style checks for heat (
>>https://blueprints.launchpad.net/heat/+spec/reduce-flake8-ignored-rules).
>>I've been already warned that enabling some of these rules will be quite
>>controversial, and personally I do not like some of these rules myself
>>either. In order to understand what is the opinion of the community, I
>>would like to ask you to leave a comment on the blueprint page about what
>>do you think about enabling these checks.
>>
>>The style rules being currently ignored are:
>>F841 local variable 'json_template' is assigned to but never used
>
>This was fixed an enabled in https://review.openstack.org/#/c/62827/
>
>>H201 no 'except:' at least use 'except Exception:' (this actually checks
>>for bare 'except:' lines, so 'except BaseException:' will pass too)
>
>This sounds reasonable, we made an effort to purge naked excepts a while
>back so hopefully it shouldn't be too difficult to enable.
>
>However there are a couple of remaining instances (in resource.py and
>scheduler.py in particular), so we need to evaluate if these are
>justifiable or need to be reworked.
>
>>H302 do not import objects, only modules (this I don't like myself as it
>>can clutter the code beyond reasonable limit)
>>H306 imports not in alphabetical order
>>H404 multi line docstring should start with a summary
>
>Personally I don't care much about any of these, in particular the import
>ones seem to me unncessesarily inconvenient so I'd prefer to leave these
>disabled.

:( I like the import checks

Ok, well whatever I guess.

My argument against the alpha-sort of imports, is that it basically
randomizes the structure from the perspective of how most people (or maybe
it's just me..) think about the imports.

I always think of imports in terms of where they come from, ie in order of
preference:

1. Import a core python module
2. Import some module available via pypi
3. Import some module from oslo sync'd to $project/openstack/common
4. Import something local to the project

This results in something like:

import functools
import json

from oslo.config import cfg
import webob

from heat.openstack.common import timeutils
from heat.openstack.common import log as logging
from heat.openstack.common import threadgroup
from heat.openstack.common.gettextutils import _
from heat.openstack.common.rpc import service
from heat.openstack.common.rpc import common as rpc_common
from heat.openstack.common import excutils
from heat.openstack.common import uuidutils

from heat.common import context
from heat.common import exception
from heat.common import identifier
from heat.common import heat_keystoneclient as hkc
from heat.db import api as db_api
from heat.rpc import api as rpc_api
from heat.engine import api
from heat.engine import attributes
from heat.engine import clients
from heat.engine.event import Event
from heat.engine import environment
from heat.engine import parameters
from heat.engine import parser
from heat.engine import properties
from heat.engine import resource
from heat.engine import resources
from heat.engine import stack_lock
from heat.engine import template as tpl
from heat.engine import watchrule

Or, if you go with the lets-alpha-sort-everything approach:

from heat.common import context
from heat.common import exception
from heat.common import heat_keystoneclient as hkc
from heat.common import identifier
from heat.db import api as db_api
from heat.engine.event import Event
from heat.engine import api
from heat.engine import attributes
from heat.engine import clients
from heat.engine import environment
from heat.engine import parameters
from heat.engine import parser
from heat.engine import properties
from heat.engine import resource
from heat.engine import resources
from heat.engine import stack_lock
from heat.engine import template as tpl
from heat.engine import watchrule
from heat.openstack.common.gettextutils import _
from heat.openstack.common import excutils
from heat.openstack.common import log as logging
from heat.openstack.common import threadgroup
from heat.openstack.common import timeutils
from heat.openstack.common import uuidutils
from heat.openstack.common.rpc import common as rpc_common
from heat.openstack.common.rpc import service
from heat.rpc import api as rpc_api
from oslo.config import cfg
import functools
import json
import webob

Which to me is not only much less readable, but also I have to expend
valuable brainpower (hey, it's a limited resource ok! :) mentally alpha
sorting every new import I add to every file.

So that's my argument against the alpha-sort OCD-ness, FWIW.

FWIW, this isn't what the hacking guidelines suggest, but the test to enforce the rest of the import ordering guidelines isn't quite in yet (it's this >< close though). Hacking actually says something similar to what you suggest, except that it requires alpha sorting the individual groups and it combines the oslo imports and the project ones. So you end up with three groups:

stdlib imports

third party lib imports

project imports (including oslo)

All of which are alpha sorted individually. Not exactly what you suggested above, but I think it's in the same spirit.

-Ben

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

Reply via email to