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

["Reshad Rahman (rrahman)" <[e-mail]>]

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" <[e-mail] on behalf of [e-mail]> wrote:

    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
    
    _______________________________________________
    netmod mailing list
    [e-mail]
    https://www.ietf.org/mailman/listinfo/netmod