Answers to the specifics of this thread here, and I will follow up with a seperate thread on the broader topic.
On Mon, Jun 23, 2014 at 7:21 AM, Ben Nemec <[email protected]> wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 06/23/2014 06:18 AM, Sean Dague wrote: > > On 06/22/2014 03:04 PM, Jay Pipes wrote: > >> On 06/20/2014 02:07 PM, Sean Dague wrote: > >>> After seeing a bunch of code changes to enforce new hacking > >>> rules, I'd like to propose dropping some of the rules we have. > >>> The overall patch series is here - > >>> > https://review.openstack.org/#/q/status:open+project:openstack-dev/hacking+branch:master+topic:be_less_silly,n,z > >>> > >>> > >>> > >>> > H402 - 1 line doc strings should end in punctuation. The real statement > >>> is this should be a summary sentence. A sentence is not just a > >>> set of words that end in a period. Squirel fast bob. It's > >>> something deeper. This rule thus isn't really semantically > >>> useful, especially when you are talking about at 69 character > >>> maximum (79 - 4 space indent - 6 quote characters). > >> > >> ++ This was always a silly rule, IMO > >> > >> Sorry, forgot to add a period. There. Better. > After looking at http://legacy.python.org/dev/peps/pep-0257/ it says The docstring is a phrase ending in a period. It prescribes the function or method's effect as a command ("Do this", "Return that"), not as a description; e.g. don't write "Returns the pathname ...".' So it looks like pep257 doesn't agree with you, and I am confused at pep257 itself. That being said since most folks aren't a fan, I am for removing it. > >> > >>> H803 - First line of a commit message must *not* end in a > >>> period. This was mostly a response to an unreasonable core > >>> reviewer that was -1ing people for not having periods. I think > >>> any core reviewer that -1s for this either way should be thrown > >>> off the island, or at least made fun of, a lot. Again, the > >>> clarity of a commit message is not made or lost by the lack or > >>> existence of a period at the end of the first line. > >> > >> +10 > >> > >> Sorry, I meant +10. Period. > >> > > I'm good with removing the punctuation ones, especially for the commit > message. > I think there is a fairly strong concensus on removing H803, it should have never landed in the first place. > > >>> H305 - Enforcement of libraries fitting correctly into stdlib, > >>> 3rdparty, our tree. This biggest issue here is it's built in a > >>> world where there was only 1 viable python version, 2.7. > >>> Python's stdlib is actually pretty dynamic and grows over time. > >>> As we embrace more python 3, and as distros start to make > >>> python3 be front and center, what does this even mean? The > >>> current enforcement can't pass on both python2 and python3 at > >>> the same time in many cases because of that. > >> > >> +0 on this one... I find it useful to group the libs together in > >> this way, as it makes it relatively easy to identify quickly at a > >> glance the third party libs that are in use in the module. > > > > Sure, I'm not saying don't group. I'm saying that it's actually > > impossible to create a formal definition for group 2. So use it as > > guidelines don't enforce in code. > > I'm -1 on manual enforcement of these rules. I hate -1'ing a patch > (especially one that's been in review for a while) just for import > grouping issues, and if I'm not going to -1 for that then why even > pretend it's something we want to do? > > I'd rather have an exception mechanism for the check where we can make > a list of modules that can be in either stdlib or third-party and > allow both. I don't think the Python stdlib is changing so fast that > we couldn't keep such a list maintained and it would allow these > checks to still be automated. In fact, it should be possible to > generate the list programmatically. > > Obviously I am signing up to do this work if it's something we can > agree on. > > Sounds like Ben just volunteered to fix this one up, thank you. So given Ben's offer to fix this up I don't think we should remove it. > > > >>> We have to remember we're all humans, and it's ok to have grey > >>> space. Like in 305, you *should* group the libraries if you > >>> can, but stuff like that should be labeled as 'nit' in the > >>> review, and only ask the author to respin it if there are other > >>> more serious issues to be handled. > >> > >> Ya, no real disagreement on that. > >> > >>> Let's optimize a little more for fun, and stop throwing -1s for > >>> silly things. :) > >> > >> ++ > >> > >> I would also love to get rid of H404, otherwise known as the dumb > >> rule that says if you have a multiline docstring, that there must > >> be a summary line, then a blank line, then a detailed > >> description. It makes things like this illegal, which, IMHO, is > >> stupid: > >> > >> def do_something(self, thing): """We do something with the > >> supplied thing, so that something else is also done with this > >> other thing. Make sure the other thing is of type something. """ > >> pass > >> > >> Likewise, I'd love to be able to have a newline start the > >> docstring, like so: > >> > >> def do_something(self, thing): """ We do something with the > >> supplied thing, so that something else is also done with this > >> other thing. Make sure the other thing is of type something. """ > >> pass > >> > >> But there's a rule that prevents that as well... > > > > Yeh, I'd be happy throwing out the docstring rules all together. I > > feel like docstrings being good or bad is pretty orthoginal to > > these rules the way they enforce things. > > > >> To be clear, I don't think all hacking rules are silly. To the > >> contrary, there are many that are reasonable and useful. However, > >> I'd prefer to focus on things that make the code more readable, > >> not less readable, and rules that enforce Pythonic idioms, not > >> some random hacker's idea of good style. > > > > Correct, I'm with you. But I also feel like we've wandered across a > > line in the current implementation, and it's time to prune back. > > > > Because hacking supports local rules, if any particular project > > wants to go overboard in rules, so be it. But lets not do that in > > hacking itself. > > I made this comment on one of the reviews too, but it would be nice if > we had a central repository for these sorts of checks. Otherwise > we're going to end up with any project that wants to enforce, say, the > docstring checks having to rewrite the checks in their local repo and > there will be a bunch of different implementations of the same rule. > > > > > -Sean > > > > > > > > _______________________________________________ OpenStack-dev > > mailing list [email protected] > > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1 > Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ > > iQEcBAEBAgAGBQJTqDfxAAoJEDehGd0Fy7uqm9UIAIsb8Pk2yyLqGzrUnPRWZNLZ > nlLoKDwMIu6iQmP6CYuGNZtxse4anUZN0Xzm5q/E90S1jyDRP4uHCTrw6xwNaQ69 > yauP37Y4pA18rv/zjABpckv/USyL0vantWOXPxnQEkZsRIUF5Lh0zehI9AkIVlwy > PfWudRX87wIfXQTjaLRW4otFt5D3udqIfinLS9LBK+2YlpZZ95htTMLBI0ZMzat8 > 5qVwPFrZ/tOW7DQmAzisYQ7C46oB9DLB2cehWZgoEk0dkLrBiEPOojRoHpPK3s0J > fIaTe/PLY5t9wSMxxJehLwkpFcu1M2CdLomLtuspXEjPpzmSsnDtSWjeMsgvMjs= > =jLGp > -----END PGP SIGNATURE----- > > _______________________________________________ > OpenStack-dev mailing list > [email protected] > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >
_______________________________________________ OpenStack-dev mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
