On Tue, 3 Jun 2014 09:29:49 -0400 (EDT) Kamil Paral <kpa...@redhat.com> wrote:
> > Outside any header requirements or directive documentation
> > requirements, are there any changes to PEP8 that folks want to
> > make? If so, please list the exceptions and why you think they
> > should be adopted.
>
> ==== E251 =====
> Josef is used to add spaces between keyword name and its value in
> method definitions or method calls. Personally, I find it more
> readable according to PEP8 (with no space), but Josef claims the
> opposite :)
Personally, I agree with Josef on this one. My habit is to put the
spaces in there and I tend to find it a bit more readable but I don't
have any really strong feelings on it and have been training myself to
not put the spaces in there.
> ==== E128 ====
>
> libtaskotron/check.py:94:17: E128 continuation line under-indented
> for visual indent libtaskotron/check.py:97:21: E128 continuation line
> under-indented for visual indent libtaskotron/check.py:209:21: E128
> continuation line under-indented for visual indent
<snip>
> Still, the most readable code I believe is the first one, against
> PEP8.
In my opinion, the first example is the least readable of the three and
I really don't like having code formatted like that.
> ==== E303 ====
>
> libtaskotron/check.py:88:5: E303 too many blank lines (2)
> This forbids you from adding two blank lines between class methods.
> Another braindead checks.
Eh, I'm not all that partial to either point of view. They're different
ways of looking at things.
> ==== E124 ====
>
> libtaskotron/bodhi_utils.py:146:25: E124 closing bracket does not
> match visual indentation
>
>
> log.warn("The build %s is a part of two different
> updates: '%s'" " and '%s'. That's probably a bug in Bodhi." % (build,
> failures[build]['title'],
> build2update[build]['title']) )
>
> Because PEP8 wants this:
>
> log.warn("The build %s is a part of two different
> updates: '%s'" " and '%s'. That's probably a bug in Bodhi." % (build,
> failures[build]['title'],
> build2update[build]['title']) )
>
> In a longer line of text, having the closing bracket really matching
> the opening one (the first example) is much easier on the eyes.
Honestly, I don't really notice things like this. I had to stare at the
two examples for a while before I realized what the difference was. I'm
fine with keeping this restriction
> ==== E122 =====
>
> libtaskotron/runner.py:127:17: E122 continuation line missing
> indentation or outdented
>
>
> raise TaskotronYamlError("Input yaml should contain
> correct 'input'" "section (a mapping). Yours was: %s" % type(
> self.taskdata['input']))
>
> I don't really understand the purpose of this error. If I move
> 'type(' to the third line, it goes away.
I agree with Josef on this one. In general, I don't like having
calls hat start on one line like that and continue onto the next line
and am fine with keeping this enabled.
> ==== E265 ====
>
> libtaskotron/directives/bodhi_comment_directive.py:422:13: E265 block
> comment should start with '# '
>
>
> #TODO: replace url with proper URL
>
>
> Me and Josef are used to avoid the space in these cases. We can
> re-learn, of course. But it's very common to write it this way.
Yeah, I had to relearn that habit as well but I'm of the opinion that
having a space between '#' and the rest of the comment does make many
comments easier to read.
> ==== E111, E113 ====
>
> libtaskotron/config_defaults.py:53:35: E111 indentation is not a
> multiple of four libtaskotron/config_defaults.py:53:35: E113
> unexpected indentation
>
>
> bodhi_posting_comments_span =
> 4320 #: # 3 days (3*24*60 = 4320)
>
>
> This is a nice example how automated style checks impede visual
> coding. In this case, it makes absolute sense to have the comment
> written in this way. However, if we suppress E111 and E113 globally,
> we might miss some potential problems elsewhere. I'm not sure if we
> can suppress certain warnings only in a selected block of code, but
> if we check for PEP8 automatically, we should find out. There will be
> more use cases like this one.
The '#:' is for making sphinx format the comments differently when
rendering docs, no? Why can't the comment explaining that 3 days is
4320 minutes be inline?
Why is making the indentation a clean multiple of 4 not an option here?
Granted, that wouldn't fix both errors but it would fix one of them.
> Sooo, I'm not exactly a big fan of automated PEP8 checking (I had to
> disable it in Spyder, because it was driving me mad), but it might be
> somewhat useful, if we configure some exceptions to the general rules.
What about using pylint instead of the pep8 tool. While I realize it
isn't 100% of what pep8 covers, they're close and AFAIK, pylint is
quite a bit more configurable than pep8 compliance checkers are.
Outside of any one (or more than one) person's dislike of particular
pep8 rules, it is a standard and there is some value in not deviating
from well-known standards too much.
Of the concerns you raised, I'm not terribly opposed to most of them
other than E128 and E122. I'm not crazy about dropping E265, though.
Tim
signature.asc
Description: PGP signature
_______________________________________________ qa-devel mailing list qa-devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/qa-devel
