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.

Sign up for GitHub

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

Jump to bottom

Rename some flake8-simplify rules #2915

Merged
merged 7 commits into from
Feb 26, 2023
Merged

Rename some flake8-simplify rules #2915

merged 7 commits into from
Feb 26, 2023

Conversation

ngnpope
Copy link
Contributor

@ngnpope ngnpope commented Feb 15, 2023 •
edited by charliermarsh
Loading

Renames the following rules that stood out to me at a glance as needing better names:

  • or-true to expr-or-true
  • and-false to expr-and-false
  • a-or-not-a to expr-or-not-expr
  • a-and-not-a to expr-and-not-expr

Related to #2902.

@ngnpope
Rename needless-bool to unnecessary-bool-call
0b9d63a 
We use `unnecessary-` in many other rule names. Also append `-call`
which makes this name structured similar to `unnecessary-list-call` as
well as better describing that it is the call to `bool()` that is not
required.
@ngnpope
Rename or-true to expr-or-true
ae2b86a
@ngnpope
Rename and-false to expr-and-false
07eb51b
@ngnpope
Rename a-or-not-a to expr-or-not-expr
dfae09d
@ngnpope
Rename a-and-not-a to expr-and-not-expr
ee9ce5e
@not-my-profile
Copy link
Contributor

not-my-profile commented Feb 15, 2023 •
edited
Loading

I disagree about renaming needless-bool, I intentionally named it that in 7e5c193 to match the clippy lint needless_bool. (Besides unnecessary-bool-call doesn't make sense to me ... what call?).

I agree that the other names you suggest are better but I would rather just merge all of these rules into a single bool-logic-bug rule, see also #2714.

@charliermarsh charliermarsh enabled auto-merge (squash) February 26, 2023 22:32
@charliermarsh
Copy link
Member

Committing all of these for now apart from needless-bool.

@charliermarsh charliermarsh merged commit 994e2e0 into astral-sh:main Feb 26, 2023
@ngnpope ngnpope deleted the rename-flake8-simplify-rules branch February 26, 2023 23:18
@ngnpope
Copy link
Contributor Author

ngnpope commented Feb 26, 2023

Thanks 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ngnpope @not-my-profile @charliermarsh