Am 25.02.2012 16:53, schrieb Alessandro Molina:
Merging the branch made me notice just one issue, for future commits
please remember to split feature/bugfix commits from pure syntax
rearrangement/cleanup ones.

You're right, clean-up work should be done in separate commits. I'm particularly guilty of this. Whenever I see a PEP8 warning anywhere in my IDE, I immediately feel compelled to "fix" it, along with the real bugfix I'm just working at, and then check in everything together. That's a bad habit of mine, I'll try to improve in this regard.

Also do we really want to enforce 80 cols PEP8 limit? I usually find
impossible to read 80cols python code and nowadays it is usually
possible to put two windows side by side even with a 110/120cols
limit. If you prefer to strictly follow PEP8 I'll adapt, but I really
think that the 80cols limit hurts a lot readability.

I think it's still useful as it's not only about screen size but also about readbility (see http://stackoverflow.com/questions/746853/the-80-column-limit-still-useful). Since Python does not use such long namespaced class names as Java, it is rare that lines are longer than 80 chars and I think it looks odd when in 100 lines 1 line "sticks out". Also, long lines often have a code smell (long methods with conditional complexity, too many parameters, excessively long identifiers). So I think the 80 col limit in PEP8 still makes sense.

But this is mainly a matter of taste. Officially, we already relaxed that rule (http://turbogears.org/2.1/docs/main/Contributing.html), I just forgot about it. It may help if we check in a .pylintrc file with e.g. max-line-length=120 and other relaxiations we like. Then all contributors can point their IDEs to this .pylintrc file, and we don't see the warnings we don't want to care about.

There are some other things that are not regulated by PEP8 and therefore are somtimes changed back and forth when different contributors prefer different styles. We should decide on one style, and document it on the page mentioned above.

E.g. we should decide how to format continuation lines.

One style is this:

def long_function_name(
        long_parameter_name, another_long_parameter_name):
    do_something()

Another one is this:

def long_function_name(long_parameter_name,
                       another_long_parameter_name):
    do_something()

Both are PEP8ish. I prefer the former, as I generally try to avoid formatting that only makes sense with monospaced chars.

While I'm at it, we should also add more and better docstrings, and enforce PEP257 in addition to PEP8:

One-line:

def rand_bar(foo):
    """Randomize the bar of a foo object."""

Multi-line:

def rand_bar(foo):
    """Randomize the bar of a foo object.

    Foo must be None, in which case we do nothing,
    or a Foo instance with a bar property.

    """

Another thing that will help is when everybody checks in with trailing whitespace stripped. I hate it when I make a small fix and then in the diff I see hundreds of lines changed because blanks have been removed at line ends by my editor. We could also use a commit hook for that.

-- Christoph

--
You received this message because you are subscribed to the Google Groups 
"TurboGears Trunk" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/turbogears-trunk?hl=en.

Reply via email to