Skip to content

feat: useless try-except #59

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

Merged
merged 1 commit into from
May 13, 2023
Merged

feat: useless try-except #59

merged 1 commit into from
May 13, 2023

Conversation

ScDor
Copy link
Contributor

@ScDor ScDor commented May 12, 2023

closes: #58

  1. I am not sure whether 3xx is the right code, wdyt?
  2. Is an auto-fix relevant in this case?

@ScDor ScDor marked this pull request as ready for review May 12, 2023 07:52
@ScDor ScDor marked this pull request as draft May 12, 2023 10:08
@ScDor ScDor marked this pull request as ready for review May 12, 2023 10:59
@guilatrova
Copy link
Owner

guilatrova commented May 12, 2023

That is a good idea.

Thanks for the suggestion and for following the commit guidelines.

I'll test it later this week/early next week and provide more feedback

@guilatrova guilatrova self-requested a review May 12, 2023 17:19
Copy link
Owner

@guilatrova guilatrova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some minor comments. Great work!

Getting to your questions:

I am not sure whether 3xx is the right code, wdyt?

Since it happens inside the except block where someone mistakenly adds a bare raise, and not inside the try block itself this new violation fits better in the 2xx group.

Is an auto-fix relevant in this case?

I don't think so, it's better to leave the developer to decide what he wanted. Maybe he should remove the try/except altogether e.g.

try: # Whole block is useless
   something()
except Exception:
   raise

Maybe he has a finally, so he would just delete the except block

try:
   something()
except Exception: # Only this block is useless
   raise
finally:
   do_something_else()

Feel free to fix these items by either:

  • Using the refactor: prefix on commits; o
  • Amending your commits to the first one

Whatever you decide is fine 👍

@ScDor ScDor requested a review from guilatrova May 13, 2023 08:23
@guilatrova guilatrova merged commit f7a56dd into guilatrova:main May 13, 2023
@guilatrova
Copy link
Owner

It was a pleasure 🤝

@ScDor ScDor deleted the useless-except branch May 19, 2023 06:02
@ScDor ScDor restored the useless-except branch May 19, 2023 06:02
@ScDor ScDor deleted the useless-except branch May 19, 2023 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New Exception Rule: Useless Try / Except statement
2 participants