Allow failing when score is affected by Information warning#5318
Allow failing when score is affected by Information warning#5318Pierre-Sassoulas merged 2 commits intopylint-dev:mainfrom
Information warning#5318Conversation
Pull Request Test Coverage Report for Build 1512268538
💛 - Coveralls |
Pierre-Sassoulas
left a comment
There was a problem hiding this comment.
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.
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.
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.
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.