[openstack-dev] [hacking] rules for removal

Joe Gordon joe.gordon0 at gmail.com
Mon Jun 23 19:06:16 UTC 2014


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 <openstack at nemebean.com> 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 OpenStack-dev at lists.openstack.org
> > 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
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140623/82e179d6/attachment.html>


More information about the OpenStack-dev mailing list