-
-
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
[deleted messages] Refactor the constant to be able to give a reason #6826
[deleted messages] Refactor the constant to be able to give a reason #6826
Conversation
Pull Request Test Coverage Report for Build 2439084107
π - Coveralls |
old_names: list[tuple[str, str]] = [] | ||
|
||
|
||
_DELETED_MSGID_PREFIXES: list[int] = [] |
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.
If we underscore this file we can then not underscore anything else in this file π
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.
Clever π
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.
Done
26eba51
to
c1405a9
Compare
c1405a9
to
19410c2
Compare
# Argparse by default allows abbreviations. It behaves differently | ||
# if you turn this off, so we also turn it on. We mimick this | ||
# if you turn this off, so we also turn it on. We mimic this |
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.
Sorry about the unrelated change but I don't know why suddenly pre-commit pylint was not happy about this.
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 must have a dictionary with both mimic and mimick.
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 might want to add to our own dictionary instead of disabling because the useless-suppression
it will get annoying fast.
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.
This is because you removed a disable
on L206.
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.
No, I had the useless suppression, thougth it was a spellcheck issue, "fixed" "mimick" (suggestion from IDE) then still had an useless-suppression, and removed the disable. I think it's the dict that has "mimick" in it on ubuntu.
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.
Let's add useless-suppression
to the disable there then. Because the other stuff in that comment still needs to be disabled.
@@ -1 +1,9 @@ | |||
bad-option-value:1:0:None:None::Bad option value for enable. Don't recognize message W04044.:UNDEFINED | |||
bad-option-value:4:0:None:None::Bad option value for disable. Don't recognize message C05048.:UNDEFINED |
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.
Do you mind doing a follow up PR just before the other PR which changes the confidence for bad-option-value
?
Please don't do it in this PR as it already quite large.
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.
Yeah sure, I already have a lot of cherry-pick to do for 2.14.1 anyway π
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.
π
All good fixes though π
@Pierre-Sassoulas This breaks |
π€ According to the primer, this change has no effect on the checked open source code. π€πThis comment was generated for commit 19410c2 |
Damn the spellcheck is fragile. It worked locally and in CI. I don't think we can do anything about this, |
When using pylint v2.14, you can get a bunch of errors like: /action/lib/.automation/.python-lint:1:0: E0015: Unrecognized option found: no-space-check (unrecognized-option) /action/lib/.automation/.python-lint:1:0: R0022: Useless option value for '--disable', 'print-statement' was removed from pylint, see pylint-dev/pylint#4942. (useless-option-value) /action/lib/.automation/.python-lint:1:0: R0022: Useless option value for '--disable', 'parameter-unpacking' was removed from pylint, see pylint-dev/pylint#4942. (useless-option-value) /action/lib/.automation/.python-lint:1:0: R0022: Useless option value for '--disable', 'unpacking-in-except' was removed from pylint, see pylint-dev/pylint#4942. (useless-option-value) Most of them have beed removed here: pylint-dev/pylint#4942 But raising exceptions when using removed options was only added in 2.14 in pylint-dev/pylint#6826 We still need to actualize this config with new options, but at least we won't get errors with this config.
Type of Changes
Description
Refactor prior to #6824 in order to be able to give a reason for why the message was deleted, and better overall organization as deleted messages are not public and specific to
pylint.messages
.