Housekeeping: Various linter fixes#3449
Conversation
* the __all__ variable should be a list of strings, but previously it was a simple string
| community_input["slug"] = "my-test-community-2" | ||
| community_input["access"]["visibility"] = "restricted" | ||
| c2 = create_community(data=community_input) | ||
| _c2 = create_community(data=community_input) |
There was a problem hiding this comment.
question: why not remove the assignment entirely?
There was a problem hiding this comment.
because that left a "hole" in the naming (c1, c3) that i think would likely create more confusion than the rename
i also toyed with the thought of removing c2 and renaming c3 to c2, but that didn't feel great either
There was a problem hiding this comment.
now i am confused, i thought all of them are not used? so c1, c2, c3 are useless variables?
There was a problem hiding this comment.
right, it seems like i mixed up the cX variables here with the other named & unused variables below (rX, draft_only, record_with_draft, ...): it's actually the removal of r2 that would cause the "hole" between r1 and r3
looking at it again, we can remove all the cX variables as they're not used; i just took the same approach for all the variables.
for the others, the names do seem to convey some sort of semantics, so i'd keep them.
| if callable(target): | ||
| target_output = redirect_url() | ||
| if type(target_output) == tuple: | ||
| if isinstance(target_output, tuple): |
There was a problem hiding this comment.
Although correct, there is a little nuance with this change: isinstance is aware of inheritance, type() == is not.
I don't expect this to cause any issues here, I'm probably just pointing it out because it has caused "unexpected behaviours" for me in the past 😅
There was a problem hiding this comment.
fair point!
i've replaced the check now with type(...) is tuple, which also made the linter happy and preserves the semantics better 😄
max-moser
force-pushed
the
mm/housekeeping
branch
from
4b16ef0 to
2a7439a
Compare
* as suggested by the linter `ruff`
* move imports to the top of the file * remove unused imports * rename unused variables
max-moser
force-pushed
the
mm/housekeeping
branch
from
2a7439a to
bba4b4e
Compare
* the removed function definition was shadowed by another one with the same name right below it
4 participants
Fix some complaints made by
ruff