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. 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'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). > Another question I have is how to proceed with such changes. I've already > implemented H306 (order of imports) and am being now puzzled with how to > propose such change to Gerrit. This change naturally touches many files > (163 so far) and as such is clearly not suited for review in one piece. The > only solution I currently can think of is to split it in 4-5-6 patches > without actually changing tox.ini, and after all of them are merged, issue > a final patch that updates tox.ini and any files breaking the rule that > were introduced in between. But there is still a question on how Jenkins > works with verify and merge jobs. Can it happen that we end up with code in > master that does not pass pep8 check? Or there will be a 'race condition' > between my final patch and any other that breaks the style rules? I would > really appreciate any thoughts and comments about this. If you do proceed with the work, then I thing those reviewing will just have to police the queue and ensure we don't merge patches which break the style rules after you've fixed them. Steve _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
