More descriptive message for use-implicit-booleaness-not-comparison
#7725
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pierre-Sassoulas
added
Documentation 📗
Skip news 🔇
Pierre-Sassoulas
changed the title
use-implicit-booleaness-not-comparisonuse-implicit-booleaness-not-comparison
Pull Request Test Coverage Report for Build 3408761029
💛 - Coveralls |
Pierre-Sassoulas
force-pushed
the
more-actionnable-message-for-use-implicit-booleaness
branch
from
a930e1f to
3377f77
Compare
|
🤖 Effect of this PR on checked open source code: 🤖 Effect on django:
The following messages are no longer emitted:
Effect on pandas:
The following messages are no longer emitted:
Effect on sentry:
The following messages are no longer emitted:
This comment was generated for commit 3377f77 |
|
Are you concerned about the first two Sentry primer messages @Pierre-Sassoulas or can we ignore/they are unrelated? |
|
It would be very surprising if no-member was affected by this change, I think it's unrelated. We probably need to set the commit we lint for each repo on the primer to avoid that. We chose not to at implementation but we're ignoring those messages now and there's never a time where we check the new false positive. If we have to update the commit in a merge request that would be the time when we check those new messages and we would not bother about it right now. |
mbyrnepr2
reviewed
Nov 9, 2022
Comment on lines
+212
to
+216
| collection_literal = { | ||
| "list": "[]", | ||
| "tuple": "()", | ||
| "dict": "{}", | ||
| }.get(description, "iterable") |
There was a problem hiding this comment.
I wonder would it be best to factor this logic out and into the _get_node_description method? It is only used in this one place. Perhaps returning a tuple of description, collection_literal? You've looked into it more than I have so no problem if you disagree.
There was a problem hiding this comment.
You're right, it does not make a lot of sense right now but I'm going to use it use-implicit-booleaness-not-len later :) (there will also be list comprehension, dict comprension...)
mbyrnepr2
approved these changes
Nov 9, 2022
Pierre-Sassoulas
deleted the
more-actionnable-message-for-use-implicit-booleaness
branch
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of Changes
Description
Follow-up to #7723 in order to make #6870 more reviewable.