search

kubernetes-sigs / structured-merge-diff

Test cases and implementation for "server-side apply"
Apache License 2.0
105 stars 62 forks source link

Remove unions logic, keep schema components #255

Closed apelisse closed 1 year ago

apelisse commented 1 year ago

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

apelisse commented 1 year ago

I tested this code along with #254 and the tests are now working.

liggitt commented 1 year ago

We keep the whole API surface intact to stay within the same major release of structured-merge-diff even though the union code will now panic if used in any place

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?

apelisse commented 1 year ago

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?

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).

liggitt commented 1 year ago

/lgtm /approve

update the title/description to match where this ended up

k8s-ci-robot commented 1 year ago

[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: - ~~[OWNERS](https://github.com/kubernetes-sigs/structured-merge-diff/blob/master/OWNERS)~~ [apelisse] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment