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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make bugbear slightly more polite #298

Merged
merged 2 commits into from Oct 19, 2022
Merged

Make bugbear slightly more polite #298

merged 2 commits into from Oct 19, 2022

Conversation

sam-w
Copy link
Contributor

@sam-w sam-w commented Oct 17, 2022

Bugbear is great at catching bugs in our code, but can come across a bit rude when commenting on pull requests 馃槀 Obviously it's a robot, but we like to encourage polite discourse

Bugbear is great at catching bugs in our code, but can come across a bit rude when commenting on pull requests 馃槀  Obviously it's a robot, but we like to encourage polite discourse
Copy link
Collaborator

@cooperlees cooperlees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fair. I鈥檇 like others thoughts if anyone comes by 鈥

bugbear.py Outdated Show resolved Hide resolved
@@ -1192,8 +1192,8 @@ def visit_Lambda(self, node):
}
B015 = Error(
message=(
"B015 Pointless comparison. This comparison does nothing but waste "
"CPU instructions. Either prepend `assert` or remove it."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally feel the blunt nature of the message makes it clearer of what the check looks for.
The new message will make people think into it more and possibly over complicate their actions. But open to others thoughts 鈥

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely see the tradeoff here, and agree that the cause of the error should be the first thing the user sees. The aim with the change of wording was to retain that, while softening the order to a suggestion.

Co-authored-by: Cooper Lees <me@cooperlees.com>
Copy link
Collaborator

@cooperlees cooperlees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Sorry about being slow. I鈥檓 on holidays at the moment.

@cooperlees cooperlees merged commit aa3c351 into PyCQA:main Oct 19, 2022
6 checks passed
@sam-w sam-w deleted the patch-1 branch October 24, 2022 00:53
@sam-w
Copy link
Contributor Author

sam-w commented Oct 24, 2022

@cooperlees no probs! Thanks for accepting the change, hope your holidays were excellent

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.

None yet

2 participants