-
Notifications
You must be signed in to change notification settings - Fork 99
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 type hints #565
base: main
Are you sure you want to change the base?
Fix type hints #565
Conversation
9fe9678
to
7c94b4e
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.
Please also change the https://github.com/databricks/databricks-sdk-py/blob/main/.codegen/__init__.py.tmpl, because your changes will get overridden by the autorelease infra
7c94b4e
to
631ba8a
Compare
I'm also noticing that this issue appear in a lot more of the codebase. I'll have a quick check through and see what else I can catch. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #565 +/- ##
=======================================
Coverage 58.08% 58.08%
=======================================
Files 45 45
Lines 29725 29726 +1
=======================================
+ Hits 17265 17266 +1
Misses 12460 12460 ☔ View full report in Codecov by Sentry. |
2a31bcc
to
dcff82a
Compare
dcff82a
to
c75094e
Compare
@nfx I think this is ready for review. Lots of changes because there were a lot of missing |
Thanks for contributing this. As you can tell, we haven't enabled type checking for the repo yet. We appreciate these fixes in the meantime. Hopefully I'll be able to work on adding type checking sometime in the next month or two. |
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.
Thanks for this! As I mentioned, we appreciate these fixes while we haven't yet set up typechecking regression testing in the repo.
Changes
Fixes: #564
Tests
make test
run locallymake fmt
applied