Open Stack

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