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.

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

Rename some flake8-simplify rules #2915

Merged
merged 7 commits into from
Feb 26, 2023

Conversation

ngnpope
Copy link
Contributor

@ngnpope ngnpope commented Feb 15, 2023

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.

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 ngnpope force-pushed the rename-flake8-simplify-rules branch from 40f091b to ee9ce5e Compare February 15, 2023 10:36
@not-my-profile
Copy link
Contributor

not-my-profile commented Feb 15, 2023

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants