-
Notifications
You must be signed in to change notification settings - Fork 63
Remove unions logic, keep schema components #255
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
I tested this code along with #254 and the tests are now working. |
is this to satisfy an APIDiff-type check, or do we really want to let someone using this code continue to compile successfully and then panic them at runtime? I can't find any uses of these symbols in kubernetes/kubernetes or on github at all (using grep.app). Would it be better to remove them? |
This removes the generally unused union code from the library in a non-backward incompatible way since the API was not used anyways. The schema isn't changed since we don't want to break existing schema (the unions fields will continue to be just ignored).
8bc0a77
to
35d4e8c
Compare
Removed them. It's obviously better to remove them but I had a hard time convincing myself I could remove these fuctions without bumping the major version which I'm not ready to do (and makes backporting the change more difficult). |
/lgtm update the title/description to match where this ended up |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: apelisse, liggitt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This removes the generally unused union code from the library in a
non-backward incompatible way since the API was not used anyways. The
schema isn't changed since we don't want to break existing schema (the
unions fields will continue to be just ignored).
cc @alexzielenski @liggitt