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. > >H404 is probably a stronger argument, as it would help improve the > >quality of our auto-generated docs, but again I see it as of marginal > >value considering the (probably large) effort involved. > > I tried to fix this a while ago and it's a big patch. I'd suggest > you break it into a bunch of patches. > > > > >I'd rather see that effort used to provide a better, more automated way to > >keep our API documentation updated (since that's the documentation users > >really need, combined with the existing template/resource documentation). > > People do what they feel comfortable with.. Yes *of course* people can spend their time doing whatever they wish, but I felt obligated to point out that these changes are mostly completely invisible to our users and the H404 change will involve considerable effort fixing something which, probably, hardly anyone will even notice. Steve _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev