[netmod] mbj review of draft-verdt-netmod-yang-module-versioning-01

[Martin Björklund <[e-mail]>]

Hi,

Here are my review comments of
draft-verdt-netmod-yang-module-versioning-01.



o  3.1.1

    o  In statements that have any data definition statements as
       substatements, those data definition substatements MAY be
       reordered, as long as they do not change the ordering or any "rpc"
       "input" substatements.

  I think this needs to capture that no descendant statements to
  "input" can be reordered.  Same for "output" (note, "input" and
  "output" in both "rpc" and "action").


o  3.3

    All revision labels that match the pattern for the "version"
    typedef in the ietf-yang-semver YANG module MUST be interpreted as
    YANG semantic version numbers.

  I don't think this is a good idea.  Seems like a layer violation.
  What if my project use another dialect of semver, that wouldn't be
  possible with this rule.  I think this needs to be removed.


o  3.3

    Submodules MUST NOT use revision label schemes that could be confused
    with the including module's revision label scheme.

  Hmm, how do I ensure that this MUST NOT is handled correctly?  What
  exactly does "could be confused with" mean?


o  3.3

      In the filename of a YANG module, where it takes the form: module-
      or-submodule-name ['@' revision-label] ( '.yang' / '.yin' )

  Should this section update 5.2 of RFC 7950?  I know that 5.2 just
  says "SHOULD".  But existing tools implement this SHOULD, and they
  need to be updated to handle this new convention.

  But I wonder if this a good idea.  It means that a tool that looks
  for a module with a certain revision date cannot simply check the
  filenames, but need to parse all available modules (wijust to find the 



o  3.4

     leaf imperial-temperature {
       type int64;
       units "degrees Fahrenheit";
       status deprecated {
         rev:status-description
           "Imperial measurements are being phased out in favor
            of their metric equivalents.  Use metric-temperature
            instead.";
       }
       description
         "Temperature in degrees Fahrenheit.";
     }

  I don't think rev:status-description is necessary / worth it.  This
  can easily be written with the normal description statement instead:

     leaf imperial-temperature {
       type int64;
       units "degrees Fahrenheit";
       status deprecated;
       description
           "Imperial measurements are being phased out in favor
            of their metric equivalents.  Use metric-temperature
            instead.

            Temperature in degrees Fahrenheit.";
     }


o  3.5

  The example modules should be legal YANG modules.  Use e.g. 
  "urn:example:module" as namespace.

  Also, the modules are missing the last "}", which confuses the
  "rfcstrip" tool.


o 4.1.1

    Alternatively, the first example could have used the revision label
    "1.0.0" instead, which selects the same set of revisions/versions.

    import example-module {
      rev:revision-or-derived 1.0.0;
    }

  Shouldn't this be s/1.0.0/2.0.0/g ?


o  5

  I think the module name "ietf-yl-revisions" should be changed to
  "ietf-yang-library-revisions".   "yl" is not a well-known acronym.


o  5.2.2

  Wouldn't it be better if the leaf "deprecated-nodes-implemented" and
  "obsolete-nodes-absent" were of type "boolean" rather than type
  "empty"?


o  7.1

  The text says:

    All IETF YANG modules MUST include revision-label statements for all
    newly published YANG modules, and all newly published revisions of
    existing YANG modules.  The revision-label MUST take the form of a
    YANG semantic version number [I-D.verdt-netmod-yang-semver].

  I strongly disagree with this new rule.  IETF modules use a linear
  history, so there are no reasons to use "modified semver".

  It is ok to use rev:nbc-changes if needed, though.


o 7.1.1

  There is a missing " in:

   4.  For status "obsolete", it is RECOMMENDED to keep the "status-
       description" information, from when the node had status
       "deprecated, which is still relevant.
 HERE  -----------^


o  8

  s/CODE ENDS>/<CODE ENDS>/


o Both YANG modules

  All extensions should specify the grammar; i.e., in which statements
  they can be present and which substatements they can have.



/martin