-----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----- _______________________________________________ OpenStack-dev mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
