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.
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. 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. 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 parent statement."; list when { NEW: grouping when-stmts { description "Describes changes of all the when statements of a specific parent statement."; list when { 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; } In Section 4, shouldn’t rpc “input” and “output” be added to the list that includes “choice” and “case”? 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. In the output module, why is module-comparison a list vs. a container? 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.) Joe
_______________________________________________ netmod mailing list -- [email protected] To unsubscribe send an email to [email protected]
