-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
add test and expl for line-too-long useless-supp FP #7887
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some wording, and I think it's better to xfail the expected behavior rather than crystalize the wrong behavior
Co-authored-by: Pierre Sassoulas <[email protected]>
Isn't the issue here that we can't know for sure that the |
Sure, in theory something else could fail, but this is a one line file with only the example, I think we're safe. There's a lot of time where we asked ourselves why something "obviously" wrong was in the functional tests. |
Can't we add: # This test serves mostly to document current behaviour.
# If you manage to make it fail and remove the useless-suppression
# warning please contact Daniel to get your
# celebratory reward (and well done ππ») |
Sounds great, what's the reward going to be though ? π |
10 internet points, a beer if they ever meet me in person and an extra speedy review of a PR of choice. |
Are you agreeing to keeping the original test, so I can revert the latest commit minus doc changes, correct? |
Sure ! |
Pull Request Test Coverage Report for Build 3605491429
π - Coveralls |
Co-authored-by: Pierre Sassoulas <[email protected]> (cherry picked from commit 30c931f)
Co-authored-by: Pierre Sassoulas <[email protected]> (cherry picked from commit 30c931f)
Co-authored-by: Pierre Sassoulas <[email protected]> (cherry picked from commit 30c931f)
Type of Changes
Description
Closes #3368