Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Improved coding standards #49

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

alquerci
Copy link
Collaborator

@alquerci alquerci commented Sep 4, 2014

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Related tickets
License GNU GPLv3
  • Talk about these conventions
  • Clean up the commit history

@nylen
Copy link
Owner

nylen commented Sep 17, 2014

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.

@alquerci
Copy link
Collaborator Author

Indeed thank you.

@nylen
Copy link
Owner

nylen commented Sep 17, 2014

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?

@alquerci
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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()

@nylen
Copy link
Owner

nylen commented Sep 17, 2014

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.

@alquerci
Copy link
Collaborator Author

  • 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   <janneke@gnu.org>
    #     Chris Cormie        <cjcormie@gmail.com>
    #     James Nylen         <jnylen@gmail.com>
    #     Alexandre Quercia   <alquerci@email.com>
    #
    # For the full copyright and license information, please view the LICENSE
    # file that was distributed with this source code.

@alquerci alquerci force-pushed the master branch 2 times, most recently from 635047d to 4be517a Compare October 7, 2014 19:59
@alquerci alquerci modified the milestones: 1.2 (master), 2.0 Apr 7, 2015
@alquerci alquerci modified the milestones: 2.0, 1.2 (master) Apr 7, 2015
@alquerci alquerci added the Minor label Apr 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants