-
Notifications
You must be signed in to change notification settings - Fork 23
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
Format python with black #280
Conversation
This includes changes from #279. |
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.
I'm a fan of using Black, even if I don't necessarily like everything that it does.
I've called out a few things below:
- Foremost, it looks like you included a commit on this branch which belongs in a separate PR.
- Beyond that, there are a couple of changes that Black made that I think you might want to resist in various ways.
- And, finally, the changes highlighted a couple of spots in the code where we might want to make some tweaks beyond what Black proposed...which you should feel free to decline.
filter( | ||
lambda x: _find_comment_in_jira(x, j_comments) is None or x['changed'] is not None, | ||
g_comments | ||
) | ||
lambda x: _find_comment_in_jira(x, j_comments) is None | ||
or x["changed"] is not None, | ||
g_comments, | ||
) |
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.
[I don't know whether you're interested in making this sort of change, but I'll point it out and you can decline if you like....]
Since we're going to include x
in the result if it is changed, we should check that before checking to see if the comment is in j_comments
.
return jira.client.JIRA( | ||
**{ | ||
"options": { | ||
"server": URL, | ||
"verify": False, | ||
}, | ||
"token_auth": TOKEN, | ||
} | ||
) |
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.
Using the **
operator on a literal is a little heavy-handed, we should do something like this, instead:
return jira.client.JIRA( | |
**{ | |
"options": { | |
"server": URL, | |
"verify": False, | |
}, | |
"token_auth": TOKEN, | |
} | |
) | |
return jira.client.JIRA( | |
options={ | |
"server": URL, | |
"verify": False, | |
}, | |
token_auth=TOKEN, | |
) |
[Again, feel free to decline if you aren't interested in this kind of change in this PR.]
Just for consistent style.