Open Stack

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 24/06/14 12:59, Joe Gordon wrote:
> Hi All,
> 
> After Friday's thread on removing several hacking rules. H402 and H803 are lines 
> up to removed in the next few days, while Ben has volunteered to work on H305. 
> In addition to helping clarify if we can remove a few specific rules, the thread 
> touched on the bigger issue of how do make sure the rules in hacking reflect the 
> communities lazy consensus of what should be enforced.
> 
> The hacking repo has consists primarily of two distinct things, HACKING.rst the 
> OpenStack Style Guide that is published at [0], and hacking the tool. Hacking 
> the style guide's goal is as Sean puts it [1]:
> 
>     "OpenStack has a set of style guidelines for clarity. OpenStack is a very
>     large code base (over 1 Million lines of python), spanning dozens of git
>     trees, with over a thousand developers contributing every 12 months. As such
>     common style helps developers understand code in reviews, move between
>     projects smoothly, and overall make the code more maintainable." 
> 
> While hacking the tool is there to move the burden of enforcing the style guide 
> off of human reviewers, as human reviewers are our most constrained resource today.
> 
> In the past when evaluating if we should add a new rule to hacking the tool, we 
> followed the  guidelines in [2]. Where consensus was met if the rule was already 
> in HACKING.rst or there was lazy consensus on this mailing list. In retrospect 
> this was a mistake, as this policy makes the assumption that the folks read 
> HACKING.rst and have done a review to decide if any sections should be changed. 
> For example we have a few unenforced sections that we may not want to keep 
> [3][4]. Going forward I propose:
> 
>   * Any addition or removal of a rule requires a ML thread and/or an oslo-specs
>     blueprint (I am not sure which one makes the most sense here, but I am
>     leaning towards the ML).
>   * Only accept new rules that are already being used as a local rule in at
>     least one repository.
>   * Add a new directory, contrib, for local rules that multiple projects use but
>     are not generally considered acceptable to be enabled by default. This way
>     we can reduce the amount of cut and pasted code (thank you to Ben Nemec for
>     this idea). 
> 
> While turning off rules at the repository level has always been recommended as 
> hacking is a tool to help projects and not a mechanism to dictate style, having 
> rules enabled in hacking does have implications. So we need a mechanism to track 
> which rules folks don't find useful so we can make sure they are not enabled by 
> default (either move them to contrib or remove them entirely). We can track 
> individual repositories hacking ignore lists and periodically reevaluate the 
> rules with the most ignores. This means projects vote on which rules to remove 
> by maintaining there ignore list in tox.ini. I put together a tiny script to do 
> this and here are my findings so far.
> 
> rule: number of ignores
> 
> H803: 19
> H302: 14
> H904: 14
> H305: 13
> H405: 12
> H307: 11
> H404: 9
> H402: 6
> H: 4
> H233: 3
> H202: 3
> H306: 2
> H301: 2
> H303: 1
> H703: 1
> H304: 1
> H237: 1
> H4: 1
> H201: 1
> H701: 1
> H102: 1
> H702: 1
> 
> Interestingly, of the two rules we just agreed to remove, H402 is not even close to the top of the most skipped list. Of the top 3 most skipped rules
> 
> 
> 
>   * H803: The first line of the commit message must not end with a period and
>     must be followed by a single blank line.
>       o Removing, as previously discussed
>   * H302: Do not import objects, only modules (*)
>       o This has been around for a long time, should we move this to contrib?
>   * H904: Wrap long lines in parentheses and not a backslash for line continuation.
>       o Although this has been in HACKING.rst for a while, the rule was added in
>         hacking 0.9. So its still unclear if this is skipped because folks don't
>         like it or because they haven't gotten around to fixing it up yet.
>         Thoughts? 

I personally like H302, but don't mind either way about the other two.
H302 requires lots of code changes so if you haven't started with it
it's a bit painful to enable that check (might be the reason people
just ignore it).

You could ditch H4*

Is there a way that "git review -s" could install the H8* rules into
a git pre commit hook locally instead of as gate checks?

- -Angus

> 
> 
> best,
> Joe Gordon
> 
> 
> [0] http://docs.openstack.org/developer/hacking/
> [1] https://review.openstack.org/#/c/102014/2/HACKING.rst
> [2] http://docs.openstack.org/developer/hacking/readme.html#adding-additional-checks
> [3] http://docs.openstack.org/developer/hacking/#dictionaries-lists
> [4] http://docs.openstack.org/developer/hacking/#calling-methods
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTqP/0AAoJEFrDYBLxZjWoZegIAK0L3ogfnFDaA5/T2UZ4gQp4
hoRJce5mIDJp99lXClQNvc/UsicTiy+kH73dGWFyxnri/xgKJT8GjRzE9OnV6xR/
SezMDfP3EYOaGoytUayX4GB7XxUmM0z6Pgd1Q4j+ckwDJIOIOgDoq8rEvMCnAuak
t4NmkBTVe+UkB8aa+KdbM/zXjCDfvq7NZYwd8j0N4Sd95Mmj3jAi+quKQnd42gJG
fdc0sPFIbH3GKNm4NsPeCo9655heS12nX5jgDW2tdhbq4JwUlw2Tb2b6C/2TlNKK
EKC2DzuvpVl4uJMdTeAWUPFQC66Nw2dfdFs0Q3RQO7Fz/CnNFccpZdDOtclONWs=
=SzCn
-----END PGP SIGNATURE-----