[openstack-dev] [hacking] community consensus and removing rules
angus.salkeld at RACKSPACE.COM
Tue Jun 24 04:34:56 UTC 2014
-----BEGIN PGP SIGNED MESSAGE-----
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 , and hacking the tool. Hacking
> the style guide's goal is as Sean puts it :
> "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 . 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
> . 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.
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?
> Joe Gordon
>  http://docs.openstack.org/developer/hacking/
>  https://review.openstack.org/#/c/102014/2/HACKING.rst
>  http://docs.openstack.org/developer/hacking/readme.html#adding-additional-checks
>  http://docs.openstack.org/developer/hacking/#dictionaries-lists
>  http://docs.openstack.org/developer/hacking/#calling-methods
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
-----END PGP SIGNATURE-----
More information about the OpenStack-dev