Opened 17 years ago

Closed 15 years ago

#1220 defect closed fixed (fixed)

Should the coding standard mention PEP8

Reported by: hypatia Owned by:
Priority: highest Milestone:
Component: core Keywords: documentation
Cc: Glyph, radix, Jean-Paul Calderone, spiv, itamarst, therve Branch:


Attachments (2)

1220.diff (856 bytes) - added by therve 15 years ago.
1220_2.diff (2.8 KB) - added by therve 15 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 17 years ago by hypatia

Adding a lot of opinionated people to the nosy list on this one, since
twisted-bugs is broken. Feel free to remove yourselves if you are opinion-free.

At least a few people think that Twisted code needs to be PEP 8 style, as per
this conversation:

<hypatia> does twisted follow pep 8?
<mumak> hypatia: not all the time. mostly though
<mumak> or maybe I'm mistaken.
<hypatia> It's not in the coding standard
<mumak> no.
<lifeless> hypatia: during the sprint, many pep8 garhs were seen and corrected -
not by me
<lifeless> hypatia: so at least some folk do ;0
<hypatia> lifeless: by whom then?
<lifeless> radix

(lifeless's "not me" thing may be inspired by working on or at least near code, which seems to turn people into raving PEP 8 folks. spiv's
been known to reformat my code PEP 8 even when that means that it would no
longer actually fit the target standard. ;) )

So, since I hate undocumented "everyone knows" policy, does Twisted code need to
be PEP8 or is our code purely subject to the coding standard as per:

<radix> hrm. Twisted doesn't follow PEP 8. The coding standard is
self-contained, and is close to a superset of PEP 8.

It seems like if that's the official line, it should go in the coding standard,
since people are starting to reference PEP 8 when criticising each other's code.

comment:2 Changed 17 years ago by Jean-Paul Calderone

If it's not in trunk/doc/core/howto/policy/coding-standard.xhtml, it's not real.
 It's entirely possible there are things which should be made real, though.

Specific suggestions?

comment:3 Changed 17 years ago by hypatia

spiv might be the person to give an overview of whether it's worth being PEP8
levels of cranky...

comment:4 Changed 16 years ago by hypatia

Cc: hypatia removed
Component: conch
Owner: changed from hypatia to edsuom

comment:5 Changed 16 years ago by Jean-Paul Calderone

Component: conchcore

Changed 15 years ago by therve

Attachment: 1220.diff added

comment:6 Changed 15 years ago by therve

Cc: therve added
Keywords: review added
Owner: edsuom deleted

I attached a simple patch mentioning PEP 8. Please review.

comment:7 Changed 15 years ago by therve

#2101 was closed as a duplicate of this.

comment:8 Changed 15 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to therve

Some conflicts which exist but which aren't actually documented in coding-standard.xhtml:

  • no backslash line continuations allowed
  • separate top-level classes and functions with three blank lines, class-level functions with two blank lines, don't ever omit the blank lines, don't ever use L
  • ASCII only (if we ever allowed non-ascii, we will allow UTF-8, not latin-1)
  • Use "as" syntax to resolve naming conflicts between different imports, not fully-qualified names (last clause in PEP8 "Imports" section)
  • The opening """ of a docstring should be on a line by itself

Some parts of coding-standard.xhtml are just out of date and wrong:

  • No "placeholder docstring" junk allowed - write as complete and descriptive a docstring as you can. If it changes later, then it changes later, no big deal.
  • The single quote/emacs thing isn't relevant anymore
Theprefix thing is wrong, code examples should be indented per epytext rules.
  • We don't update ChangeLog anymore, the release manager does NEWS at release time
  • In the "Methods" section, the example of how to add multiple callbacks is terrible and wrong.

The first set of these should probably be fixed before we add the PEP8 fallback. The second set can be addressed separately.

Changed 15 years ago by therve

Attachment: 1220_2.diff added

comment:9 Changed 15 years ago by therve

Keywords: review added
Owner: therve deleted
Priority: highhighest

Thanks for the review, I've addressed the first points in the last patch. The second set should be done in #2217.

comment:10 Changed 15 years ago by Jonathan Lange

Keywords: review removed
Owner: set to therve
  • Regarding long lines, we cannot use parentheses to wrap long lines in import statements while we support Python 2.3. I believe our standard is to repeat the import, e.g.
from very.long.package import foo, bar, baz
from very.long.package import qux, quux, quuux
  • The second diff hunk says "musn't", change to "must not".

Once these are changed, please land this branch.

comment:11 Changed 15 years ago by therve

Resolution: fixed
Status: newclosed

(In [21396]) Make the coding standard mention PEP8 as fallback, and resolve the conflicts between the two.

Author: therve Reviewers: jml, exarkun Fixes #1220

comment:12 Changed 11 years ago by <automation>

Owner: therve deleted
Note: See TracTickets for help on using tickets.