-
-
Notifications
You must be signed in to change notification settings - Fork 680
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
some pylint tidyups #502
some pylint tidyups #502
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,8 +7,7 @@ def maybe_create_user(username: str): | |
if username in existing_usernames: | ||
print("The user already exists") | ||
raise typer.Exit() | ||
else: | ||
print(f"User created: {username}") | ||
print(f"User created: {username}") | ||
Comment on lines
7
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Throughout the docs, the house style is to use |
||
|
||
|
||
def send_new_user_notification(username: str): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ | |
def hello(count, name): | ||
"""Simple program that greets NAME for a total of COUNT times.""" | ||
for x in range(count): | ||
click.echo("Hello %s!" % name) | ||
click.echo(f"Hello {name}!") | ||
Comment on lines
7
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While F-strings are typically nicer and more readable, there are a few cases where you might prefer not using them. When we're logging things for instance, you might prefer avoiding the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah! This one is just that it was an exact copy of Click's tutorial. But they now updated it to use f-strings, so I would take this change. 🤓 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cf #891 |
||
|
||
|
||
if __name__ == "__main__": | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When there's multiple
if
statements (implicit here because of the chaining) combined with a negation, I personally find keeping the parentheses more clear & readable - so I would vote to revert this change.