-
-
Notifications
You must be signed in to change notification settings - Fork 12
Add linters and auto formatter #5
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
Conversation
benmod
left a comment
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.
All good except those imports in the init.py file that the linter wrongly thought were unused but are actually used by anybody using the library.
| @@ -1,9 +1,5 @@ | |||
| from .token_encode import OpenPAYGOTokenEncoder | |||
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 removing those imports? They are left at top level so that you can access them importing just the module, if we remove them it will break existing tools using it.
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 raised "unused variable" warnings. I think the recommended approach is to use __all__. Let me try to adapt. Do you have a piece of code, where I can test?
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.
Nothing public but basically you should be able to import all of those variables when importing openpaygo. Like from openpaygo import TokenType should work.
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 personally find it a bit odd, if from openpaygo import TokenType works, but from openpaygo import * and then openpaygo.TokenType doesn't. Using __all__ should be non-breaking. WDYT?
benmod
left a comment
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.
All good now!
Apply'ing
blackandruffto the repository.Doing this in a first step, such that we can add the merge commit to
.git-blame-ignore-revsfile in a second step, see https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-viewAfter that, adding CI to check linting of Pull Requests.
Note: This should be merged using Squash&Rebase