-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 06/16/2014 12:01 PM, Sean Dague wrote: > On 06/16/2014 12:44 PM, Ben Nemec wrote: >> On 06/16/2014 08:37 AM, Thierry Carrez wrote: >>> Sean Dague wrote: >>>> Hacking 0.9 series was released pretty late for Juno. The >>>> entire check queue was flooded this morning with requirements >>>> proposals failing pep8 because of it (so at 6am EST we were >>>> waiting 1.5 hrs for a check node). >>>> >>>> The previous soft policy with pep8 updates was that we set a >>>> pep8 version basically release week, and changes stopped >>>> being done for style after first milestone. >>>> >>>> I think in the spirit of that we should revert the hacking >>>> requirements update back to the 0.8 series for Juno. We're >>>> past milestone 1, so shouldn't be working on style only fixes >>>> at this point. >>>> >>>> Proposed review here - >>>> https://review.openstack.org/#/c/100231/ >>>> >>>> I also think in future hacking major releases need to happen >>>> within one week of release, or not at all for that series. >> >>> We may also have reached a size where changing style rules is >>> just too costly, whatever the moment in the cycle. I think it's >>> good that we have rules to enforce a minimum of common style, >>> but the added value of those extra rules is limited, while >>> their impact on the common gate grows as we add more projects. >> >> A few thoughts: >> >> 1) I disagree with the proposition that hacking updates can only >> happen in the first week after release. I get that there needs >> to be a cutoff, but I don't think one week is reasonable. Even >> if we release in the first week, you're still going to be dealing >> with hacking updates for the rest of the cycle as projects adopt >> the new rules at their leisure. I don't like retroactively >> applying milestone 1 as a cutoff either, although I could see >> making that the policy going forward. >> >> 2) Given that most of the changes involved in fixing the new >> failures are trivial, I think we should encourage combining the >> fixes into one commit. We _really_ don't need separate commits >> to fix H305 and H307. This doesn't help much with the reviewer >> load, but it should reduce the gate load somewhat. It violates >> the one change-one commit rule, but "A foolish consistency..." > > The challenge is that hacking updates are basically giant merge > conflict engines. If there is any significant amount of code > outstanding in a project, landing hacking only changes basically > means requiring much of the outstanding code to rebase. > > So it's actually expensive in a way that doesn't jump out > immediately. The cost of landing hacking isn't just the code of > reviewing the hacking patches, it's also the cost of the extra > roundtrips on outstanding patches.
If a project has that many violations of a new hacking rule then I'd say they should just leave it disabled. It already happens and I'd rather have that than throw out a rule just because some projects haven't been following it. > >> 3) We should start requiring specs for all new hacking rules to >> make sure we have consensus (I think oslo-specs is the place for >> this). 2 +2's doesn't really accomplish that. We also may need >> to raise the bar for inclusion of new rules - while I agree with >> all of the new ones added in hacking .9, I wonder if some of them >> are really necessary. > >> 4) I don't think we're at a point where we should freeze hacking >> completely, however. The import grouping and long line wrapping >> checks in particular are things that reviewers have to enforce >> today, and that has a significant, if less well-defined, cost >> too. If we're really going to say those rules can't be enforced >> by hacking then we need to remove them from our hacking >> guidelines and start the long process of educating reviewers to >> stop requiring them. I'd rather just deal with the pain of >> adding them to hacking one time and never have to think about >> them again. I'm less convinced the other two that were added in >> .9 are necessary, but in any case these are discussions that >> should happen in spec reviews going forward. > > I think both of those cases are really nits to the point that they > aren't worth enforcing. They won't change the correctness of the > code. And barely change the readability. Yeah, I suppose that's my personal dislike of \ line continuations talking. I don't like the idea of dropping our import guidelines though - I've seen code that doesn't follow them and it's not nice to read. Of course, I'm horribly biased on that too since I wrote the check. Then again, it's something we've been enforcing manually all along, so in theory it shouldn't be that difficult to fix either. > > There are differences with things like the is None checks, or > python 3 checks, which change correctness, or prevent subtle bugs. > But I think we're now getting to a level of cleanliness enforcement > that trumps functionally working. > >> 5) We may want to come up with some way to globally disable pep8 >> checks we don't want to enforce, since we don't have any control >> over that but probably don't want to just stop updating pep8. >> That could make the pain of these updates much less. > > Actually, if you look at python novaclient, the doc string checks, > which are really niggly, and part of hacking are the biggest fails. > It's much less upstream that's doing this to us. Sure, and I believe I raised that concern when the docstring check was first proposed so I'm not going to argue too strongly for it. I do think it's a worthwhile thing if it's not too difficult to enable, but if a project is going to have a huge number of docstrings to fix then they just shouldn't enable the check. > >> I could probably come up with a few more, but this is already >> too wall-of-texty for my tastes. :-) >> >> -Ben -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTn1w9AAoJEDehGd0Fy7uqSpEH/2deuXIqVKFgwqJyw4Sde02l khhikZUiUOgjggC8N3YxZHlCl7fSonZCuDrcKmIN5QtskZGgyrIiydJ2oavJsvdQ jJhvJdDMbvOVU6UAQs0MoXTWSPjkFLLduQOv5aMpLKnxbGiQfCcc+R+rznixDdK3 EKtyVD6QMFG+PPVxy+catqqD8KA1S+YUa8mXK+keI23KTmflGY/CTf5x+fye9vsZ MMv+hVL1RUqQIqh1HyJqMxzoTUinmLqxLZtbI1sqvYnOkHml/jt1ETRxPsB/S3Wk sZUOb0PFwcI8FaauZbVqJ1m7K7Xm293IreXuD9CdyvmKOlGOk7jR3tkgxyVU3Gw= =K2X9 -----END PGP SIGNATURE----- _______________________________________________ OpenStack-dev mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
