cyg-apt icon indicating copy to clipboard operation
cyg-apt copied to clipboard

[WIP] Improved coding standards

Open alquerci opened this issue 11 years ago • 7 comments

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Related tickets
License GNU GPLv3

  • [x] Talk about these conventions
  • [ ] Clean up the commit history

alquerci avatar Sep 04 '14 19:09 alquerci

I was adding some inline notes but then I realized it'll be easier for everybody if I make a branch with some language fixes.

nylen avatar Sep 17 '14 16:09 nylen

Indeed thank you.

alquerci avatar Sep 17 '14 16:09 alquerci

I have no problem with any of these. Here's my branch with a few English fixes: https://github.com/nylen/cyg-apt/tree/pr-49-proofreading

I noticed you put a space before the colon in if True : so do we want to document that in the coding standards too?

nylen avatar Sep 17 '14 16:09 nylen

I noticed you put a space before the colon in if True : so do we want to document that in the coding standards too?

Indeed

alquerci avatar Sep 17 '14 17:09 alquerci

For control structures there MUST be one space between the expression and the colon.

The if structure can like like:

  • 1a

    if True :
        # if body
    
  • 1b

    if (True) :
        # if body
    

Without parentheses looks better?

When the conditional part of an if-statement is long enough to require that it be written across multiple lines. In this case it need parentheses.

  • 2a

    if (
        True
        and True
    ) :
        # if body
    
  • 2b

    if (True
        and True
    ) :
        # if body
    
  • PEP 8

    # No extra indentation.
    if (this_is_one_thing and
        that_is_another_thing):
        do_something()
    
    # Add a comment, which will provide some distinction in editors
    # supporting syntax highlighting.
    if (this_is_one_thing and
        that_is_another_thing):
        # Since both conditions are true, we can frobnicate.
        do_something()
    
    # Add some extra indentation on the conditional continuation line.
    if (this_is_one_thing
            and that_is_another_thing):
        do_something()
    

alquerci avatar Sep 17 '14 17:09 alquerci

Let's go with 1a because 1b does not look Pythonic to me.

I don't have a strong opinion on 2, but I'd probably prefer the second PEP8 style. I usually leave a blank line between the conditional and the statement, like this:

if this_is_one_thing
    and that_is_another_thing:

    do_something()

A comment instead of a blank line is better. If you have a complicated conditional, then you probably need a comment to explain it.

nylen avatar Sep 17 '14 18:09 nylen

  • To be consistent with sequence types the 2a should be the best option.

  • Multi-lines sequence types declaration should be look like that IMHO:

    # list
    foo = [
        "bar",
        "baz",
    ];
    
    # dict
    foo = {
        'key': "value",
        'bar': "baz",
    };
    
    # function / method declaration
    def foo(
        *args,
        **kargs,
    ):
        # body
    
    # function / method call
    foo(
        *args,
        **kargs,
    );
    
  • The colon usage is not consistent between control structure, function / method / class declaration, dict declaration, slicing a[i:j], lambda lambda: and some keywords try except finally else ... Finally to be consistent with it there is three solution:

    • 3a: There MUST not have white space before a colon;
    • 3b: There MUST have one space before a colon and at least one white space after it;
    • 3c: Do not consistent with it and introduce some exceptions;
  • For delimiting a string the simple quote should only be used and use double quote into it for quoting:

    'foo "bar" baz'
    

    Because with the current convention each time we need to use a string we ask what quote to choose (this is it an identifier or not?).

  • We also need to choice how writing a multi-line string.

  • Simplify the license block by removing useless lines and years. The specific years should be only on the LICENSE file.

    # This file is part of the cygapt package.
    #
    # (c) Jan Nieuwenhuizen   <[email protected]>
    #     Chris Cormie        <[email protected]>
    #     James Nylen         <[email protected]>
    #     Alexandre Quercia   <[email protected]>
    #
    # For the full copyright and license information, please view the LICENSE
    # file that was distributed with this source code.
    

alquerci avatar Sep 29 '14 20:09 alquerci