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

Linter: function parameters and line breaks #18

Open
JasonMWhite opened this issue Mar 9, 2017 · 5 comments
Open

Linter: function parameters and line breaks #18

JasonMWhite opened this issue Mar 9, 2017 · 5 comments

Comments

@JasonMWhite
Copy link
Member

When a function declaration uses type hints and needs to break across lines to stay within the character limit, we should have one parameter per line.

e.g.

def foo(self,
        bar: int,
        baz: str,
        qux: typing.Dict[str, typing.List[int]]) -> bool

    return True

and not:

def foo(self, bar: int, baz: str,
        qux: typing.Dict[str, typing.List[int]]) -> bool
    return True

As discussed https://github.com/Shopify/data_bang/pull/97#discussion_r105196063

@erikwright @honkfestival @cfournie does that about sum it up?

@honkfestival
Copy link

and needs to break across lines to stay within the character limit

I might even go so far as to say when it has more than two (2) typed parameters.

@JasonMWhite
Copy link
Member Author

JasonMWhite commented Mar 9, 2017

I'm ambivalent. @cfournie care to weigh in?

@cfournie
Copy link
Contributor

When using the Python 2 function type annotations, matching names to types becomes difficult after two typed parameters and one parameter and comment per line would alleviate that difficulty.

Using the Python 3 annotations I agree more with using one-param per line after 2 types parameters after reading these examples:

def foo(self,
        bar: int,
        baz: str,
        qux: long) -> bool
    return True
def foo(self, bar: int, baz: str, qux: long) -> bool
    return True

@erikwright
Copy link
Contributor

Those examples are pretty compelling, @cfournie .

@JasonMWhite
Copy link
Member Author

Then we're agreed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants