Open Stack

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.
   - Removing, as previously discussed
   - H302: Do not import objects, only modules (*)
      - 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.
      - 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?


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140623/fc12254c/attachment.html>