-
Notifications
You must be signed in to change notification settings - Fork 18
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
fix!: restrict exported names with __all__ #306
Conversation
Should |
7bc1df2
to
da8836d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #306 +/- ##
==========================================
- Coverage 97.10% 94.84% -2.27%
==========================================
Files 27 20 -7
Lines 1176 659 -517
==========================================
- Hits 1142 625 -517
Misses 34 34
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
da8836d
to
b30a492
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.
nice work 💪
940e537
to
9113338
Compare
I think exposing |
9113338
to
c5b7d05
Compare
Signed-off-by: Federico Bond <federicobond@gmail.com>
c5b7d05
to
93de1c7
Compare
Something weird with the Codecov uploader, @toddbaert @beeme1mr do you know what might be going on? |
I think this happens when Codecovs attempts to call the GitHub APIs and are throttled. I just ran one of the failed tests to see if that fixed it. If that doesn't help, I will investigate tomorrow. |
Unfortunately, it looks like a very common issue. |
We should probably only run Codecov once after the python specific tests finish. |
Yeah, there is no need to upload the results for each Python version, just one is enough. We can add an if condition to only upload for Python 3.11 as it is the default version for dinner other jobs in the same workflow. |
Signed-off-by: gruebel <anton.gruebel@gmail.com>
ok, seems like we are not the only ones with the codecov issue codecov/codecov-action#1359 seems like we need to upgrade to v4 with API token. |
Unfortunately, that has challenges too. Perhaps should should consider replacing CodeCov or skipping the upload step for now. |
@beeme1mr an alternative could be https://coveralls.io/ thoughts? |
I inclined to not rush with the decision and just ignore upload failures in CI temporarily. |
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Refs #214
Note: this is a breaking change. We should do an ecosystem check to make sure most packages are using the correct imports.