kubernetes-sigs / structured-merge-diff

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

typed: Update compare algorithm to handle duplicates #251

Closed apelisse closed 11 months ago

apelisse commented 12 months ago

Same strategy as #249, still trying to address #234. Algorithm goes as follow:

  1. Duplicates are treated as separate from non-duplicates items
  2. Adding a duplicate to an existing item means we will remove the existing (counted as removed) and added as a duplicate (added).
  3. Removing does the opposite
  4. If any of the duplicated value changes, key will be counted as "modified".

Rewording:

In compare, treat duplicate keys in associative lists/sets as a separate atomic list, marked by owning just the duplicated key.

/assign @alexzielenski /cc @jpbetz

k8s-ci-robot commented 12 months 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 12 months ago

OK, I've added comments to explain what happens, hopefully that helps.

apelisse commented 11 months ago

OK I've added a new commit to create a new map of PEs to interface{}, and then updated the other commit to use it instead of value, which simplifies the code a tiny bit since we don't have to create the new valueinterface thingy.

alexzielenski commented 11 months ago

/lgtm /hold

in case you wanted to change anything else

apelisse commented 11 months ago

/hold

I'm good thanks!

apelisse commented 11 months ago

I meant /hold cancel