Re: [yang-doctors] YANG Doctor Review of draft-ietf-i2rs-yang-network-topo-03

["Susan Hares" <[e-mail]>]

Kent: 

 

Thank you for the Yang Doctor reviews.  The authors will respond to your comments shortly. 

 

Sue 

 

From: Kent Watsen [mailto:[e-mail]] 
Sent: Tuesday, July 5, 2016 2:55 PM
To: [e-mail]
Cc: YANG Doctors
Subject: YANG Doctor Review of draft-ietf-i2rs-yang-network-topo-03

 

Hi,

 

I am the assigned YANG doctor for this individual submission document.  This review regards the YANG modules alone.  While I did read the draft, I only did so to better understand the modules.   Note: I was hoping the draft would contain examples, but alas it does not.

 

This draft contains two yang modules: ietf-network and ietf-network-topology

    - ietf-network-topology imports and augments ietf-network

    - both yang modules pass `pyang --ietf` testing.

 

These modules are relatively small and straightforward, which makes reviewing them easy.  Below are some detailed comments on these two modules.

 

ietf-network

==========

·         grouping “node-ref” is defined but not used in this module, though it is used by the ietf-network-topology module.  Should the “node-ref” grouping should be moved to the ietf-network-topology module?

·         Why have top-level container “networks-state”?   Why not have the “server-provided” config false leaf in /nd:networks/nd:network/ instead?   

·         Is there a need to define any notifications in this module?  - or is the plan to leverage yang-push?

 

ietf-network-topology

==================

·         The description statements for “link-id” and “tp-id” were not helpful.  The node is a URI, so I expect the description statement to be mostly about setting the URI value.  Example URI values would be helpful.

·         Watch capitalization on the “link” description statement: “A Network Link connects a by Local (Source) node and a Remote (Destination) Network Nodes.”

·         The sentence structure of the first sentence in the “link” description statement needs fixing.

·         Using uncommon acronyms is discouraged.  Recommend replacing “tp” with “termination-point” throughout.

·         Is the description statement for the grouping link-ref missing the word “to”?

·         The description for “supporting-termination-point” says “leaf list”, though it’s actually just a list...

·         I notice that ‘require-instance false’ is used on all leafrefs except source/dest-node and source/desy-tp, is this intentional?

 

 

Kent