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
        > 