[openstack-dev] [Heat] Reducing pep8 ignores
Sean Dague
sean at dague.net
Wed Jan 22 12:25:07 UTC 2014
On 01/22/2014 06:23 AM, 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
> H201 no 'except:' at least use 'except Exception:' (this actually checks
> for bare 'except:' lines, so 'except BaseException:' will pass too)
> H302 do not import objects, only modules (this I don't like myself as it
> can clutter the code beyond reasonable limit)
Realize you can do import aliases.
import sqlalchemy as sa
That looks to be best practice in the python community right now.
> H306 imports not in alphabetical order
> H404 multi line docstring should start with a summary
>
> 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.
As long as it is all done in a git patch series on your side, with the
patches stacked on top of each other in the correct order, it will be fine.
The system that we have won't let you merge a pep8 error, you are
protected from that.
When I was doing similar cleanups for nova last year I just ended up
with a 17 deep patch queue, which tended to merge in chunks of 4 then
need rebasing, as something else landed and changed in front of me.
-Sean
--
Sean Dague
Samsung Research America
sean at dague.net / sean.dague at samsung.com
http://dague.net
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 482 bytes
Desc: OpenPGP digital signature
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140122/6a00cff2/attachment.pgp>
More information about the OpenStack-dev
mailing list