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

From: Martin Björklund <[email-masked]>
Date: Tue, 10 Mar 2020 20:30:27 +0100 (CET)

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

---

From: "Reshad Rahman (rrahman)" <[email-masked]>
Date: Fri, 20 Mar 2020 21:07:57 +0000

Hi Martin,

We've opened issues to track your review comments (see below). Will kick off separate therads for each issue.

https://github.com/netmod-wg/yang-ver-dt/issues?q=is%3Aissue+is%3Aopen+label%3Aupdated-mod-rev-handling

Regards,
Reshad.

On 2020-03-10, 3:31 PM, "netmod on behalf of Martin Björklund" <[email-masked] on behalf of [email-masked]> wrote:

    Hi,

    
    o  3.1.1

    o  3.3

    o  3.3

    o  3.3

    

    o  3.4

    

    o  3.5

    o 4.1.1

        import example-module {

    o  5

    o  5.2.2

    o  7.1
    
      The text says:

    

    o 7.1.1
    
      There is a missing " in:

    o  8

    o Both YANG modules

    
    /martin
    
    _______________________________________________
    netmod mailing list
    [email-masked]
    https://www.ietf.org/mailman/listinfo/netmod