Skip to content
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

Test cases for new InstanceNode.merge() method #122

Merged
merged 2 commits into from
Nov 4, 2022

Conversation

kwatsen
Copy link
Contributor

@kwatsen kwatsen commented Nov 3, 2022

Hi Lada,

There are two test cases:

  • no-op: merges self into self, asserts no diff.
  • simple: merges a tweaked-self into self, asserts expected diff.

Additionally, I integrated into my project and it seems to be working okay...albeit, the project test-cases just uses the Jukebox data-model and data-set from RFC 8040. :laugh:

It is still my opinion that:

  • leaf-lists should be merged
  • ordered-by user [leaf-]lists should be collated
  • bits should be merged

That is, ConfD got it right, libyang got it half-right, YumaPro and now Yangson got it wrong.

If you're willing to update to match ConfD's approach, I'm happy to provide another PR. Otherwise, I concede that there is no definition, it's your project, and it doesn't seem to matter in practice (Andy said)...

Thanks for helping to make this update happen!

Best regards,
Kent

@cznic-knotbot cznic-knotbot merged commit c971e16 into CZ-NIC:merge Nov 4, 2022
@llhotka
Copy link
Member

llhotka commented Nov 4, 2022

Merged, thanks.

  • leaf-lists should be merged

Yes, this is what sec. 7.7.9 in RFC 7950 says, so I changed the method behaviour for leaf-lists.

  • ordered-by user [leaf-]lists should be collated

I think they are: remaining entries from value argument are appended at the end. Or do you mean something else by "collated"?

  • bits should be merged

We could change this too, but sec. 7.6.7 says quite clearly:

  • If the operation is "merge" or "replace", the node is created if it does not exist, and its value is set to the value found in the XML RPC data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants