-
Notifications
You must be signed in to change notification settings - Fork 21
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
refactor!: simplify namespaces to make public API more pythonic #172
Conversation
Codecov Report
@@ Coverage Diff @@
## main #172 +/- ##
==========================================
- Coverage 93.47% 92.93% -0.54%
==========================================
Files 23 16 -7
Lines 460 439 -21
==========================================
- Hits 430 408 -22
- Misses 30 31 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more 📢 Have feedback on the report? Share it here. |
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.
This looks great, thank you! Please make sure to update the examples on the readme.
I'll review this first thing next week. Thanks @federicobond |
Signed-off-by: Federico Bond <federicobond@gmail.com>
Signed-off-by: Federico Bond <federicobond@gmail.com>
Signed-off-by: Federico Bond <federicobond@gmail.com>
Signed-off-by: Federico Bond <federicobond@gmail.com>
Signed-off-by: Federico Bond <federicobond@gmail.com>
Signed-off-by: Federico Bond <federicobond@gmail.com>
a99328e
to
7f3df9a
Compare
👍 |
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.
LGTM!
Obviously this is a breaking change, but I have no issue merging it. I'll do so in the next few days unless I hear objections from the other (more Python-literate) reviewers. |
For reviewing, it's best to follow the changes commit by commit.
This reduces the number of public namespaces in the sdk, following what we agreed on #101
I consolidated some modules into a single file when it made sense. The alternative would have been to prefix the internal namespace files with underscores and re-export the relevant symbols from
__init__.py
but it added a lot of boilerplate with little benefit. If the modules grow in size to make this necessary, it can be done later without breaking compatibility with existing users.