-
Notifications
You must be signed in to change notification settings - Fork 102
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
Add Typeshare support for Python #169
Add Typeshare support for Python #169
Conversation
Thanks to the prior work of @adriangb: 1Password#25
Doing some review, sorry for the delay. I'd like to ask for a high-level description in this MR for how this integration works, with especial focus on how structs are handled (in particular how the JSON <-> struct boundary is handled). |
f4540ab
to
85634d4
Compare
85634d4
to
48a8adb
Compare
fbfde61
to
5c88f19
Compare
c901b1b
to
9935aaf
Compare
As a subsequent follow up, one idea for a way to handle unions would resemble this:
I'm still a novice around pydantic, so there could certainly be a better way that I'm not aware of. For all I know this pattern is fully built-in to pydantic somewhere and I just don't know where. |
After some experimentation and discussion with @MOmarMiraj, I think this is the best approach for externally tagged enums. I've refactored the PR to do so. |
Hey all! I'm ready to approve once the remaining feedback is address (or marked Will Not Fix) |
…culea/typeshare into hculea/add-python-support-in-typeshare
For future improvement, let me know if I've missed anything.
|
Thanks @CheatCod! I've captured these in #217 to make sure we don't lose track of them once merging this PR. I've resolved all the outstanding conversations after making sure they have either been logged as a future improvement or actually resolved. @Lucretiel I think this should be ready to go! Let me know if you have any other questions or concerns 👍 |
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.
Left some comments about multiple file generation support.
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.
Sorry I didn't approve before I left for the long weekend!
This PR is adding initial support for Python in Typeshare (gated by a feature flag, like Go).
See @MOmarMiraj 's comment for a high-level description of the design considerations behind this work.
This was made possible thanks to the prior work of @adriangb: #25.