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