Fix FBT001 Boolean-typed positional argument in function definition #1765
Conversation
|
@danlamanna FYI, I thought you'd dislike this, but I actually realize it's a typically a good practice to encourage boolean arguments to be keyword-only. |
brianhelba
force-pushed
the
boolean-arg
branch
from
fd1e0c0
to
b38b588
Compare
| @@ -5,7 +5,7 @@ | |||
|
|
|||
|
|
|||
| def _normalize_version_metadata( | |||
| raw_version_metadata: dict, embargo: bool, name: str, email: str | |||
| raw_version_metadata: dict, embargo: bool, name: str, email: str # noqa: FBT001 | |||
There was a problem hiding this comment.
Why exclude this one? Because these arguments (including embargo) constitute a uniform and required description of the data to be normalized?
There was a problem hiding this comment.
@waxlamp Good point. I was just being conservative. Changing this would either require making multiple other arguments keyword-only or changing the order of the arguments. It's an internal API, so it wouldn't break anything to change it, however.
Now they you mentioned it, I'm inclined to change this one too, particularly to have another example of the "right" (really just "safer") way to have boolean arguments.
This explicitly expects Django signals and Click options to be called with keyword arguments.
brianhelba
force-pushed
the
boolean-arg
branch
from
b38b588
to
e970122
Compare
|
I agree with this. I think scrutinizing any usage of boolean function arguments is a good thing, and requiring boolean args to be keyword-only is a good middle-ground. |
|
🚀 PR was released in |
This explicitly expects Django signals and Click options to be called with keyword arguments.