Again, thanks for the review, Mahesh. I noticed after I sent you the reply you didn’t copy netmod. I’ve posted -23 to address many of your concerns. On the six author front, while we could possibly move people to contributor, all of the authors listed have spent significant time on this document at various stages, so although there is a soft limit of 5 authors, it might be reasonable to make an exception in this case.
https://author-tools.ietf.org/iddiff?url2=draft-ietf-netmod-yang-semver-23 Joe From: Mahesh Jethanandani <[email protected]> Date: Tuesday, August 5, 2025 at 17:46 To: [email protected] <[email protected]> Subject: AD review of draft-ietf-netmod-yang-semever-20 (Document 2 of 3) Hi Authors, Thanks for working on this document. Just like the other document, it has taken a lot to get here. The document is well written and easy to read. While this review is particularly for this draft, the two other related drafts, draft-ietf-netmod-yang-module-versioning and draft-ietf-netmod-yang-module-filename, will be progressed along with this draft as a single cluster. Please find enclosed some comments that hopefully go towards improving the document. This document updates RFC8407, RFC8525, and RFC7950, but does not seem to include explanatory text about this in the abstract. Please add. Paragraph 0 > Network Working Group J. Clarke, Ed. > Internet-Draft R. Wilton, Ed. > Updates: 8407, 8525, 7950 (if approved) Cisco Systems, Inc. > Intended status: Standards Track R. Rahman > Expires: 11 August 2025 Equinix > B. Lengyel > Ericsson > J. Sterne > Nokia > B. Claise > Huawei > 7 February 2025 This review is of -20 version of the document. The authors have published two more versions since. Will review them separately and provide comments if needed. Can a justification for having six authors on the front page be provided? Or have one or two people hold the editor pen, and move everyone to the Contributor section?? Section 6, paragraph 0 > This section and the IETF-specific sub-section below provide YANG > Semver-specific guidelines to consider when developing new YANG > modules. As such this section updates [RFC8407]. Should this refer to rfc8407bis? Also, can it refer to a particular section of the referenced document? Section 6.1, paragraph 0 > All published IETF modules and submodules MUST use YANG semantic > versions in their revisions. This is a more direct example of how this document updates RFC 7950. The update part should be called out here. Section 7.1.1, paragraph 0 > The ietf-yang-library-semver YANG module augments the "module" and > "submodule" lists in ietf-yang-library with "version" leafs to > optionally declare the version identifier associated with each module > and submodule. The optionality of the version identifier in ietf-yang-library seemingly contradicts the requirement stated in Section 6.1 that all IETF modules and submodules MUST use a YANG semantic version. I can understand that a revision statement is optional in the YANG library. But if a revision statement is provided for a module, or a submodule, should a semantic version statement also not be provided? Section 8, paragraph 12 > description > "The version extension can be used to assign a version number > to a module or submodule revision. Not sure why the use of “can”. Is the “can” because there is another method to assign a version number to a module or submodule? Or is the “can” because assigning a version is optional? Why not a stronger statement like “is” or even “MUST”? Section 8, paragraph 14 > Adding a version is a backwards-compatible > change. Changing or removing an existing version in > the revision history is a non-backwards-compatible > change, because it could impact any references to that > version."; I can understand why removing an existing version statement may be considered non-backwards-compatible. But why changing? What if the update is to another backwards-compatible version? Section 8, paragraph 19 > Adding, removing or updating a 'recommended-min-version' > statement to an import is a backwards-compatible change."; Same comment here as above. Section 8, paragraph 20 > This YANG module contains the augmentations to the ietf-yang-library. Please follow the practice of calling out all RFCs referenced in the YANG module here, outside the module, and part of the normative text, so that idnits can make sure all RFCs have been correctly referenced (even though in this case RFC8525 has been called out elsewhere). Section 8, paragraph 32 > revision 2025-01-21 { > ysv:version "0.20.0"; > description > "Initial revision"; > reference > "RFC XXXX: YANG Semantic Versioning"; > } Is there a version of pyang that validates this module without error? Currently, it gives this error: $ pyang [email protected] [email protected]:67: error: syntax error: expected separator, got: ": ..." $ pyang --version pyang 2.6.1 Section 11, paragraph 0 > The YANG module specified in this document defines a schema for data > that is designed to be accessed via network management protocols such > as NETCONF [RFC6241] or RESTCONF [RFC8040]. The lowest NETCONF layer > is the secure transport layer, and the mandatory-to-implement secure > transport is Secure Shell (SSH) [RFC6242]. The lowest RESTCONF layer > is HTTPS, and the mandatory-to-implement secure transport is TLS > [RFC8446]. Please make sure this template matches the template defined in rfc8407bis. Section 12.2, paragraph 1 > IANA is responsible for maintaining and versioning some YANG modules > and submodules, e.g., iana-if-types.yang [IfTypeYang] and iana- > routing-types.yang [RoutingTypesYang]. Why some? I thought IANA is responsible for maintaining and versioning all IANA YANG modules that have a registry associated with them? Using lowercase "not" together with an uppercase RFC2119 keyword is not acceptable usage. Found: "MUST not" ------------------------------------------------------------------------------- NIT ------------------------------------------------------------------------------- All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. Section 6, paragraph -1 > All modules and submodules being developed or updated that use the > YANG Semver versioning scheme in MUST have unique YANG Semver version > identifiers throughout their lifecycle. s/scheme in MUST/scheme MUST/ Section 6.1.3, paragraph 0 > Whether a new IETF module is being define, or a previously published > IETF module is being updated, special considerations are required > when multiple parallel competing drafts are in progress for a new > module or a module update. s/is being define/is being defined/ These URLs point to tools.ietf.org, which has been taken out of service: * http://tools.ietf.org/wg/netmod/ Section 4.4.1, paragraph 2 > the versions could look like, from oldest version to newest: 0.1.0 - first p > ^^^^^^ A determiner may be missing. Section 5.2, paragraph 20 > Whether a new IETF module is being define, or a previously published IETF mo > ^^^^^^ Consider using either the past participle "defined" or the present participle "defining" here. "I", paragraph 27 > oned with a linear history, it is anticipated that it should not be necessar > ^^^^^^^^^^^^^^ You have used the passive voice repeatedly in nearby sentences. To make your writing clearer and easier to read, consider using active voice. Section 13.1, paragraph 2 > ents relative time (date & time) that an module is being published. Module ve > ^^ Use "a" instead of "an" if the following word doesn't start with a vowel sound, e.g. "a sentence", "a university". Section 13.1, paragraph 8 > -Q Figure 6: Scenario 2 For each revisions N, P and Q, the following are poss > ^^^^^^^^^ The noun should probably be in the singular form. Thanks Mahesh Jethanandani [email protected]
_______________________________________________ netmod mailing list -- [email protected] To unsubscribe send an email to [email protected]
