search

kubernetes-sigs / structured-merge-diff

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

tofieldset: Add tests to show it already allow duplicates #249

Closed apelisse closed 1 year ago

apelisse commented 1 year ago

Continuation of #247.

This uses the pattern that I've generally used to solve the problem, which is to not try to be too smart about duplicate entries, and just record the path element for the whole thing, rather than try to track all the fields within. It's not super relevant because no one should ever be able to apply duplicates, so they wouldn't really have conflicts with sub-fields anyway, we've decided that they should have a conflict with the idea of "duplicates" (de-duplicating is a conflict).

/assign @alexzielenski

k8s-ci-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: apelisse

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
apelisse commented 1 year ago

/hold

I'm trying a new test with pruning fields that are removed by appliers and this doesn't work, looking into this.

apelisse commented 1 year ago

/hold cancel

Thought about this further, I think it's fine, unless I forgot the use-case I had in mind ...

alexzielenski commented 1 year ago

LGTM from me. I think you mentioned you wanted @jpbetz input so will defer tag

k8s-ci-robot commented 1 year ago

@alexzielenski: Closed this PR.

In response to [this](https://github.com/kubernetes-sigs/structured-merge-diff/pull/249#issuecomment-1751335894): >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
k8s-ci-robot commented 1 year ago

@alexzielenski: Reopened this PR.

In response to [this](https://github.com/kubernetes-sigs/structured-merge-diff/pull/249#issuecomment-1751336006): >/reopen Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
alexzielenski commented 1 year ago

whoops. brain misfire

/lgtm