-
-
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
rename broad-except
and new check broad-exception-raised
#7709
Conversation
Pull Request Test Coverage Report for Build 3400111474
💛 - Coveralls |
This comment has been minimized.
This comment has been minimized.
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 looks pretty good and polished at first glance 👌
results look reasonable, my question about the
or other forms of disabling |
broad-except
and new check broad-exception-raised
That's the use case for old_names :) |
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.
Look good apart for the unrelated confidence change :) We also probably need to regenerate the doc following the removal of a message (tox -e docs
or cd doc;make build
)
pylint/checkers/exceptions.py
Outdated
"raising-bad-type", | ||
node=self._node, | ||
args="tuple", | ||
confidence=INFERENCE, |
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.
Why inference and not high? Also it should be done in another MR.
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.
It's in a sep commit but np, I can split that out. Also, I'm pretty sure I traced all the code / funcs that received inference results, but I could of course have made a mistake. Can move it to a sep PR for discussion.
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.
Thank you !
5ed6135
to
5407c2d
Compare
This comment has been minimized.
This comment has been minimized.
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.
@@ -0,0 +1 @@ | |||
You can help us make the doc better `by contributing <https://github.com/PyCQA/pylint/issues/5953>`_ ! |
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 there's still a problem here following resolution of conflict for the merge of main I just did. This new file should not exists. Would you mind rebasing on main and cleaning up the doc generation commit by doing a partial add of only the broad-except-x related changes ? Sorry for the mess, we did not have time to fix #6736 yet or this would not be a problem.
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, I think the latest looks most accurate, let's see how CI reacts
This comment has been minimized.
This comment has been minimized.
a977ae7
to
c51f18f
Compare
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.
Pretty clean check and refactor, thank you 👌
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.
Only looked at the pyreverse
change, but that looks good to me!
Just a suggestion for the future. Please split it into two separate PRs. I would consider a new check a larger enough change that the separation would probably be better, even if it's related to the refactoring. |
This commit updates pylintrc configuration files used for pylint. It removes a bunch of messages that don't exist anymore or updates their name in cases when they were renamed. Checks removed were either: * renamed * applied to python 2 * are now performed by other tools * applied to really old versions of odoo Some cases which have official documentation are the following: * bad-continuation and bad-whitespace were removed in pylint 2.6 https://pylint.pycqa.org/en/v2.11.1/whatsnew/2.6.html * bad-optional-value was split into useless-option-value and unknown-option-value https://pylint.pycqa.org/en/latest/user_guide/messages/error/bad-option-value.html * missing-docstring split into three checks (module,class,function) https://pylint.pycqa.org/en/latest/user_guide/messages/convention/missing-docstring.html * no-self-use is redundant since the extension that creates it is not used * no-init was removed pylint-dev/pylint#6373 * useless-super-delegation was renamed to useless-parent-delegation * broad-except renamed and a new check added: pylint-dev/pylint#7709
This commit updates pylintrc configuration files used for pylint. It removes a bunch of messages that don't exist anymore or updates their name in cases when they were renamed. Checks removed were either: * renamed * applied to python 2 * are now performed by other tools * applied to really old versions of odoo Some cases which have official documentation are the following: * bad-continuation and bad-whitespace were removed in pylint 2.6 https://pylint.pycqa.org/en/v2.11.1/whatsnew/2.6.html * bad-optional-value was split into useless-option-value and unknown-option-value https://pylint.pycqa.org/en/latest/user_guide/messages/error/bad-option-value.html * missing-docstring split into three checks (module,class,function) https://pylint.pycqa.org/en/latest/user_guide/messages/convention/missing-docstring.html * no-self-use is redundant since the extension that creates it is not used * no-init was removed pylint-dev/pylint#6373 * useless-super-delegation was renamed to useless-parent-delegation * broad-except renamed and a new check added: pylint-dev/pylint#7709
* [REF] cfg: bump pylint-odoo to v9.0.4 pylint v3.0.0 was released and pylint-odoo has been updated to work with it. A new major version of pylint-odoo has therefore been released. BREAKING CHANGE: support for python < 3.7 (everything below py3.8) has been dropped since pylint no longer supports it upstream either. * [FIX] core: simplify != 0 comparisons New pylint 3.0.0 release was raising use-implicit-booleaness-not-comparison-to-zero errors. They have been corrected. * [REF] cfg: make `use-implicit-boleaness-*` optional These checks were previously part of a pylint extension so they had to be manually loaded as plugins. They now form part of the core and simply need to be enabled. After internal discussion it was decided to make them optional. * [REF] cfg: drop dead messages from pylintrc Messages that were not generated by pylint-odoo anymore were removed from the respective configuration files to reduce clutter. * [IMP] cfg: update autofix hooks This commit updates the version for all hooks in the autofix configuration to their latest release. It also moves isort's configuration on the .yaml itself to the respective configuration file for the tool since that is the prefered way. * [IMP] cfg: update optional hooks & flake config This commit updates the version of all optional hooks to their latest release. flake8-bugbear which flake8 uses has been updated as well. flake8 configuration was updated as well, since this new version does not support inline commnets. B018 which checks for useless expressions was also supressed on __manifest__.py files. * [IMP] cfg: update mandatory hook versions & flake config All the versions for hooks in .pre-commit-config.yaml, known as mandatory hooks, have been updated. flake8 configuration had to be modified since this specific project has two config sources, setup.cfg and flake8, the [flake8] section in setup.cfg had to be removed since it was overriding the one set in .flake8. Also removed the exclude line, since some other messages may apply to __init__.py, and __unported__ files wont even be checked because pre-commit excludes them (not a python file). * [IMP] tests: validate pylintrc files A test has been added to verify there are no duplicate messages in a pylintrc enable/disable section and that all messages actually exist. This makes it easier to keep clean pylintrc files by automating some basic checks on them. * [FIX] cfg: remove duplicate message in .pylintrc files * [REF] cfg: cleanup pylintrc, remove non-existent messages This commit updates pylintrc configuration files used for pylint. It removes a bunch of messages that don't exist anymore or updates their name in cases when they were renamed. Checks removed were either: * renamed * applied to python 2 * are now performed by other tools * applied to really old versions of odoo Some cases which have official documentation are the following: * bad-continuation and bad-whitespace were removed in pylint 2.6 https://pylint.pycqa.org/en/v2.11.1/whatsnew/2.6.html * bad-optional-value was split into useless-option-value and unknown-option-value https://pylint.pycqa.org/en/latest/user_guide/messages/error/bad-option-value.html * missing-docstring split into three checks (module,class,function) https://pylint.pycqa.org/en/latest/user_guide/messages/convention/missing-docstring.html * no-self-use is redundant since the extension that creates it is not used * no-init was removed pylint-dev/pylint#6373 * useless-super-delegation was renamed to useless-parent-delegation * broad-except renamed and a new check added: pylint-dev/pylint#7709 * [REF] cfg: (flake8) disable B905,B907 and make E741 optional * [REF] pylintrc: make optional * [REF] flake8: ignore E501 This check was not being generated as mandatory previously, therefore it has been ignored. It is already covered by other checks anyways. * [REF] cfg: update pylint-odoo to v9.0.4 * [REV]: keep black in same version This keeps the performance improvement of using a mypyc compiled version with no new style changes, those will be applied at a later time.
Type of Changes
Description
This PR renames
broad-except
tobroad-exception-caught
:becomes
and adds a new checker,
broad-exception-raised
Closes #7494