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.

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

CHORE: Do not use pass in an except statement. #4518

Merged
merged 17 commits into from
Apr 18, 2024
Merged

Conversation

MaxJPRey
Copy link
Collaborator

No description provided.

@ansys-reviewer-bot
Copy link
Contributor

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@codecov-commenter
Copy link

codecov-commenter commented Apr 17, 2024

Codecov Report

Attention: Patch coverage is 13.04348% with 20 lines in your changes are missing coverage. Please review.

Project coverage is 76.64%. Comparing base (3b95612) to head (4e92f26).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4518      +/-   ##
==========================================
- Coverage   81.58%   76.64%   -4.95%     
==========================================
  Files         110      110              
  Lines       53563    53563              
==========================================
- Hits        43698    41051    -2647     
- Misses       9865    12512    +2647     

@SMoraisAnsys
Copy link
Collaborator

There seem to be an issue introduced by one of the last commits.
Since I'm also doing some refactoring, I adopted the follow approach about logging. If the method/function concerned is not public, I log in debug, else I use info. Since you mainly use error level, which approach do you think we should follow ?

@MaxJPRey
Copy link
Collaborator Author

MaxJPRey commented Apr 17, 2024

There seem to be an issue introduced by one of the last commits. Since I'm also doing some refactoring, I adopted the follow approach about logging. If the method/function concerned is not public, I log in debug, else I use info. Since you mainly use error level, which approach do you think we should follow ?

What issue did you notice @SMoraisAnsys ?

@SMoraisAnsys
Copy link
Collaborator

There seem to be an issue introduced by one of the last commits. Since I'm also doing some refactoring, I adopted the follow approach about logging. If the method/function concerned is not public, I log in debug, else I use info. Since you mainly use error level, which approach do you think we should follow ?

What issue did you notice @SMoraisAnsys ?

I was speaking about the following:
image
Everything seems fine now

pyaedt/desktop.py Outdated Show resolved Hide resolved
MaxJPRey and others added 3 commits April 18, 2024 08:23
Co-authored-by: Kathy Pippert <84872299+PipKat@users.noreply.github.com>
Co-authored-by: Kathy Pippert <84872299+PipKat@users.noreply.github.com>
Co-authored-by: Kathy Pippert <84872299+PipKat@users.noreply.github.com>
Co-authored-by: Kathy Pippert <84872299+PipKat@users.noreply.github.com>
pyaedt/desktop.py Outdated Show resolved Hide resolved
@MaxJPRey
Copy link
Collaborator Author

There seem to be an issue introduced by one of the last commits. Since I'm also doing some refactoring, I adopted the follow approach about logging. If the method/function concerned is not public, I log in debug, else I use info. Since you mainly use error level, which approach do you think we should follow ?

What issue did you notice @SMoraisAnsys ?

I was speaking about the following: image Everything seems fine now

OK. Got it. Thanks @SMoraisAnsys

@MaxJPRey
Copy link
Collaborator Author

Thanks @PipKat for all the suggestions. All approved and/or committed.

When a non public method triggers a log in a try: except: pass pattern, we use DEBUG level instead of ERROR or INFO to avoid confusing users.
Copy link
Collaborator

@SMoraisAnsys SMoraisAnsys left a comment

Choose a reason for hiding this comment

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

LGTM

@MaxJPRey MaxJPRey enabled auto-merge (squash) April 18, 2024 09:46
@MaxJPRey MaxJPRey merged commit d27d9a7 into main Apr 18, 2024
15 checks passed
@MaxJPRey MaxJPRey deleted the chore/pass_except branch April 18, 2024 12:07
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.

5 participants