Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Rename bad-exception-context to bad-exception-cause #3694

Closed
cool-RR opened this issue Jun 16, 2020 · 1 comment · Fixed by #7125
Closed

Rename bad-exception-context to bad-exception-cause #3694

cool-RR opened this issue Jun 16, 2020 · 1 comment · Fixed by #7125
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors
Milestone
2.15.0

Comments

@cool-RR
Copy link
Contributor

cool-RR commented Jun 16, 2020

While working on #3687 , I noticed bad naming. The bad-exception-context rule needs to be renamed to bad-exception-cause, because it's about the cause and not the context.

Note that the only way that the part after from could be related to the __context__ of the exception, is if it's None, but:

  1. Our rule is only relevant when it's not None.
  2. Even when it's None, the __context__ is still the same, it's just the __suppress_context__ attribute that gets set to True.

We should also rename the _check_bad_exception_context method accordingly, and its docstring.
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.15.0 milestone Jul 1, 2022
@Pierre-Sassoulas Pierre-Sassoulas added the Needs decision 🔒 Needs a decision before implemention or rejection label Jul 1, 2022
@DanielNoord
Copy link
Collaborator

Seems like a good idea? It creates a bit of churn but could be a nice PR for a new contributor?

@Pierre-Sassoulas Pierre-Sassoulas added Help wanted 🙏 Outside help would be appreciated, good for new contributors Good first issue Friendly and approachable by new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation Enhancement ✨ Improvement to a component and removed Needs decision 🔒 Needs a decision before implemention or rejection Discussion 🤔 labels Jul 2, 2022
Fifi31 pushed a commit to Fifi31/pylint that referenced this issue Jul 4, 2022
Rename bad-exception-context to bad-exception-cause
899d796 
Fixes pylint-dev#3694
@Fifi31 Fifi31 mentioned this issue Jul 4, 2022
Merged
@Pierre-Sassoulas Pierre-Sassoulas removed the Needs PR This issue is accepted, sufficiently specified and now needs an implementation label Jul 4, 2022
Pierre-Sassoulas added a commit that referenced this issue Jul 18, 2022
@Fifi31 @Pierre-Sassoulas
Rename bad-exception-context to bad-exception-cause (#7125)
bc247dc 
Fixes #3694

Co-authored-by: Victor Taïx <victor.taix@grenoble-inp.org>
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors
Projects
None yet
Milestone
2.15.0
Development

Successfully merging a pull request may close this issue.

Rename bad-exception-context to bad-exception-cause Fifi31/pylint
3 participants
@cool-RR @Pierre-Sassoulas @DanielNoord