<div dir="ltr"><p>Hi All,</p><p>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.</p>

<p>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]: </p><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

"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." </blockquote>

<p>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.</p>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:<br>

<ul><li>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).</li><li>Only accept new rules that are already being used as a local rule in at least one repository.</li>

<li>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).</li>

</ul><p>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.</p>

<pre style="margin-top:0px;margin-bottom:0px;padding:5px 0px"><pre style="color:rgb(0,0,0);font-family:'Bitstream Vera Sans Mono',monospace;font-size:13px;margin-top:0px;margin-bottom:0px;padding:5px 0px">rule: number of ignores
<pre style="margin-top:0px;margin-bottom:0px;padding:5px 0px;font-family:'Bitstream Vera Sans Mono',monospace">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
</pre></pre><pre style="color:rgb(0,0,0);font-family:'Bitstream Vera Sans Mono',monospace;font-size:13px;margin-top:0px;margin-bottom:0px;padding:5px 0px"><span style="font-family:arial;font-size:small;color:rgb(34,34,34)">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</span></pre>

<pre style="margin-top:0px;margin-bottom:0px;padding:5px 0px"><ul style><li style><span style="color:rgb(34,34,34);font-family:arial;font-size:small">H803: </span><font face="arial">The first line of the commit message must not end with a period and must be followed by a single blank line.</font><font color="#000000" face="Bitstream Vera Sans Mono, monospace" size="3"><br>

</font></li><ul><li style><font face="arial">Removing, as previously discussed</font></li></ul><li style><span style="color:rgb(34,34,34);font-family:arial;font-size:small">H302: </span><font face="arial">Do not import objects, only modules (*)</font></li>

<ul><li style><font face="arial">This has been around for a long time, should we move this to contrib?</font></li></ul><li style="color:rgb(0,0,0);font-size:13px"><font face="arial, helvetica, sans-serif">H904: Wrap long lines in parentheses and not a backslash for line continuation.</font></li>

<ul><li style="color:rgb(0,0,0);font-size:13px"><font face="arial, helvetica, sans-serif">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?</font></li>

</ul></ul></pre></pre><p><br></p>best,<br>Joe Gordon<p><br></p>[0] <a href="http://docs.openstack.org/developer/hacking/">http://docs.openstack.org/developer/hacking/</a><div>[1] <a href="https://review.openstack.org/#/c/102014/2/HACKING.rst">https://review.openstack.org/#/c/102014/2/HACKING.rst</a><br>

[2] <a href="http://docs.openstack.org/developer/hacking/readme.html#adding-additional-checks">http://docs.openstack.org/developer/hacking/readme.html#adding-additional-checks</a><br>[3] <a href="http://docs.openstack.org/developer/hacking/#dictionaries-lists">http://docs.openstack.org/developer/hacking/#dictionaries-lists</a><br>

[4] <a href="http://docs.openstack.org/developer/hacking/#calling-methods">http://docs.openstack.org/developer/hacking/#calling-methods</a>
</div></div>