Re: [yang-doctors] Yangdoctors last call review of draft-ietf-ospf-yang-09

Hi Martin, 
We believe that https://datatracker.ietf.org/doc/draft-ietf-ospf-yang/ satisfies your comments. Please take a look.
Thanks,
Acee

On 12/6/17, 6:46 AM, "Martin Bjorklund" <mbj@tail-f.com> wrote:

    Ladislav Lhotka <lhotka@nic.cz> wrote:
    > Reviewer: Ladislav Lhotka
    > Review result: Ready with Issues
    > 
    > The data model defined in this document is a massive piece of work: it
    > consists of 11 YANG modules and defines around 1200 schema nodes. The

    11 YANG modules?  Isn't there just one?

    I would like to add two quick comments to Lada's review.

    o  Remove all "revision" statements except the one that says "initial
       revision".   The idea is to have one revision statement per
       published version (i.e., RFC).

    o  Many of the nodes have quite rudimentary descriptions, and the
       "reference" statement is rarely used.  For example:

             leaf lsa-id {
               type yang:dotted-quad;
               mandatory true;
               description "LSA ID.";
             }

       It seems as the description is put there just to keep the tools
       quiet.  I know that it is painful to write good descriptions, but
       it does help the consumer of the data model.

    /martin

    > "ietf-ospf@2017-10-30" module is compatible with the NMDA architecture.
    > 
    > **** Comments
    > 
    > 1. Unless there is a really compelling reason not to do so, the
    >    "ietf-ospf" should declare YANG version 1.1. For one,
    >    "ietf-routing" that is being augmented by "ietf-ospf" already
    >    declares this version. Some of my suggestions below also assume
    >    version 1.1.
    > 
    > 2. The "ietf-ospf" can work only with the new NMDA-compatible
    >    revisions of some modules, such as "ietf-interfaces" and
    >    "ietf-routing". I understand it is not desirable to import such
    >    modules by revision, but at least it should be mentioned in a
    >    description attached to every such import.
    > 
    > 3. Maybe the draft could mention that implementations should supply a
    >    default routing domain as a system-controlled resource.
    > 
    > 4. In "when" expressions, the module uses literal strings for
    >    identities. This is known to be problematic, the XPath functions
    >    derived-from() or derived-from-or-self() should be used instead.
    > 
    > 5. Some enumerations, such as "packet-type" and "if-state-type"
    >    define enum identifiers with uppercase letters and/or underscores,
    >    for example "Database-Description" or "LONG_WAIT". RFC6087bis
    >    recommends that only lowercase letters, numbers and dashes. I think
    >    this convention should be observed despite the fact that the
    >    current names are traditionally used in OSPF specs. The
    >    "ietf-routing" module also defines "router-id" even though the
    >    documents use "Router ID".
    > 
    > 6. The types of LSA headers are modelled as integers. While OSPF gurus
    >    probably know these numbers by heart, it is not very
    >    reader-frienly. So at least some references to documents defining
    >    these numbers should be provided, but my suggestion is to consider
    >    implementing them with identities. It seems it might also be useful
    >    to define some "abstract" identities for these types. For example,
    >    if "opaque-lsa" is defined, then the definition of container
    >    "opaque" could simply use
    > 
    >      when "derived-from(../../header/type, 'ospf:opaque-lsa')";
    > 
    >    instead of
    > 
    >       when "../../header/type = 9 or "
    >               + "../../header/type = 10 or "
    >               + "../../header/type = 11";
    > 
    > 7. The title of sec. 2.9 should be "OSPF Notifications" rather than
    >    "OSPF notification".
    > 
    > 
    > _______________________________________________
    > yang-doctors mailing list
    > yang-doctors@ietf.org
    > https://www.ietf.org/mailman/listinfo/yang-doctors
    > 