-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Allow failing when score is affected by Information
warning
#5318
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
Conversation
Pull Request Test Coverage Report for Build 1512268538
π - Coveralls |
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.
I think we need to keep the exit_code that can be analyzed bitwise. Ie : https://www.google.com/search?client=firefox-b-d&q=pylint+exit+code (first result, even included in their research page by google). I thought a little about it and I think going to 64 by adding 32 for I message would break a lot of things script and doc and stackoverflow answers included. So I guess the best option would be to consider enabled I message like if they were C messages. Do you think of something else ?
This would make the whole point of bit-wise encoding the exit code pointless for I and C messages. I don't think we should do this.
Why don't we add I as 64 though? If people are using the exit codes correctly this shouldn't matter. It breaks the consistency, but would work otherwise. |
It's true that the correct use is with |
I'll add a commit that 1) changes |
@Pierre-Sassoulas Thought about it again, but I don't think we can do this. Taking the example from the I think the change in this PR is still an option. We do it in another place as well, and if somebody sets the |
We could design this like mypy and ignore the discrepancies between python interpreters : pylint-dev/astroid#1248 (comment) |
sys.exit(0) | ||
else: | ||
# We need to make sure we return a failing exit code in this case. | ||
# So we use self.linter.msg_status if that is non-zero, otherwise we just return 1. |
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.
I'm not sure about the 1
here. Is it really necessary ? Shouldn't it be 32 because if self.linter.msg_status is 0 then there is only Informational messages ?
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.
We do the same on L414. 32 corresponds to usage error
so that also doesn't make much sense. We don't have a good code for Information
..
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.
Fair enough, it's another issue entirely.
Type of Changes
Description
Based on work in #5303
Not really worth a changelog entry imo. Nobody will have run into this problem, but it might be useful for ourselves at some point.