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

Uniformizing error message and error message formatting #889

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jaesivsm
Copy link
Contributor

@jaesivsm jaesivsm commented May 5, 2022

After launching GTG from flatpak I got an ill formatted error (showing %s instead of the proper values). It's due to a raise ValueError in GTG/core/config.py. After looking at that I started just fixing and uniformizing the error message formatting across GTG. Here's what's in the PR :

  • uniformizing error message formating :
    • using the same string formatting mechanism
  • raise NotImplementedError not NotImplemented which isn't an exception
>>> raise NotImplemented
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: exceptions must derive from BaseException
  • do not explicitly reraise current error in an except: block, juste use bare raise statement
  • raise error objects, not classes

Edit: encountered same error than #877

After launching GTG from flatpak I got an ill formatted error (showing `%s` instead of the proper values). It's due to a `raise ValueError` in `GTG/core/config.py`. After looking at that I started just fixing and uniformizing the error message formatting across GTG. Here's what's in the PR :
* uniformizing error message formating :
  * using the same string formatting mechanism
* raise `NotImplementedError` not `NotImplemented` which isn't an exception
```
>>> raise NotImplemented
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: exceptions must derive from BaseException
```
* do not explicitly reraise current error in an `except:` block, juste use bare `raise` statement
* raise error objects, not classes
@jaesivsm jaesivsm force-pushed the fix/uniformizing-raising branch from 4fb85e0 to d04ad6c Compare May 5, 2022 15:33
Copy link
Contributor

@Neui Neui left a comment

Choose a reason for hiding this comment

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

Haven't explicitly tested, but seems fine to me.

@nekohayo
Copy link
Member

Bonjour François, please rebase to current master if possible.

@nekohayo nekohayo marked this pull request as draft February 26, 2024 18:40
@nekohayo nekohayo added the maintainability Automated tests suite, tooling, refactoring, or anything that makes it easier for developers label Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainability Automated tests suite, tooling, refactoring, or anything that makes it easier for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants