Hi Joe,thanks for the detailed review, it is appreciated and sorry for my delay in response, I got to look at it only now.
On 7. 5. 2026 18:22, Joe Clarke (jclarke) wrote:
I had some time to read through the latest revision of this draft and focus on more than just the algorithm and how it compares to my pyang work. Thanks, Michal and Per for working on this.I have suggested this several months ago and such descriptions were deemed unnecessary, in general. So, I have included them in the schema-comparison module because I disagree and consider a clear clarification and definition important.One overarching comment is I wonder if the descriptions for NBC, BC, and Editorial shouldn’t be left to YANG Module Versioning and YANG Semver. Those drafts do tackle those meanings (though I recognize your description of editorial could help flesh out what is in YANG Semver).
And, as we discussed on our call, I think the extension definitions should be separate from the output module. They can be in the same document, but there should be two different YANG modules.
Right, working on this.
Now, on to specific feedback. There are some issues with the IANA considerations: OLD: This document register one URI in the "ns" subregistry of the IETF XML Registry [RFC3688] maintained at https://www.iana.org/assignments/xml-registry/xml-registry.xhtml#ns. Following the format in [RFC3688], the following registration is requested: URI: urn:ietf:params:xml:ns:yang:ietf-schema-comparison Registrant Contact: The IESG. XML: N/A, the requested URI is an XML namespace. NEW: This document registers one URI in the "ns" subregistry of the IETF XML Registry [RFC3688] maintained at https://www.iana.org/assignments/xml-registry/xml-registry.xhtml#ns. Following the format in [RFC3688], the following registration is requested: URI: urn:ietf:params:xml:ns:yang:ietf-yang-schema-comparison Registrant Contact: The IESG. XML: N/A, the requested URI is an XML namespace.
Right, will be fixed in the next draft.
Section 6: Typo in YANG module description for when-stmts grouping OLD: grouping when-stmts { description"Describes changes of all the must statements of a specific parentstatement."; list when { NEW: grouping when-stmts { description"Describes changes of all the when statements of a specific parentstatement."; list when {
Fixed in the next draft.
Section 6: The description for the length/restriction leaf within the length container says "Range argument substatement value.”OLD: container length { description "Length statement substatements."; leaf restriction { type string; description "Range argument substatement value."; } uses restriction-substmts; } NEW: container length { description "Length statement substatements."; leaf restriction { type string; description "Length argument substatement value."; } uses restriction-substmts; }
Fixed in the next draft.
In Section 4, shouldn’t rpc “input” and “output” be added to the list that includes “choice” and “case”?
No, these nodes are actually compiled and are compared.
In Section 4.3.3, what about: OLD: Examples of a less common or obvious compatibility breaking changes can be changing the "yang-version" of a YANG module or expanding a "type" size. The former change may break importing modules because of specific rules about "import" statement and the target module YANG version. Changing a "type" of a leaf from "uint16" to "uint32" may seem to fit the definition of a backwards-compatible change but it may not be so for some data encodings that use a fixed binary data chunk that would grow in size or even a text format that may use a different encoding for the 2 types. Specifically, the JSON YANG data format encodes a "uint32" value as a number but a "uint64" as a string. NEW: Examples of less common or less obvious compatibility-breaking changes are changing the "yang-version" of a YANG module or expanding the size of a "type". The former change may break importing modules because of specific rules about the "import" statement and the target module's YANG version. Changing the "type" of a leaf from "uint32" to "uint64" may seem to fit the definition of a backwards-compatible change, but it may not be so for some data encodings that use a fixed binary data chunk (which would grow in size) or even a text format that may use a different encoding for the two types. Specifically, the JSON YANG data format [RFC7951] encodes a "uint32" value as a JSON number but a "uint64" value as a JSON string.The latter couches the example you had in the actual problem description plus references where this is defined.
Okay, the NEW text used in the next draft version.
In the output module, why is module-comparison a list vs. a container?
It creates a new instance for some direct module substatement changes such as "prefix", "contact" and so on.
In your descriptions for presence in node-substmts and in general for
“inverted”, I think you could use a bit more text. What about the
following:
OLD:
leaf presence {
type boolean;
description
"Presence substatement existence.";
}
NEW:
leaf presence {
type boolean;
description
"Indicates whether a presence substatement is defined for
the container in the compiled schema. The textual argument
of 'presence' is reported via the parsed schema comparison
(see the 'presence' leaf in 'parsed-substmts').";
}
OLD:
leaf inverted {
type empty;
description
"Inverted substatement value.";
}
NEW:
leaf inverted {
type empty;
description
“A value of true indicates that the pattern's 'modifier'
substatement is 'invert-match' (see RFC 7950
Section 9.4.6).";
}
(Or use a reference statement there.)
Okay, improved in the next draft version.I have just got to updating the draft with a few more additional changes so a new version will not be available at least for a few more days.
Regards, Michal
Joe _______________________________________________ netmod mailing list [email protected] To unsubscribe send an email [email protected]
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________ netmod mailing list -- [email protected] To unsubscribe send an email to [email protected]
