Subject: Re: [mpls] Yangdoctors early review of draft-ietf-mpls-base-yang-10

From: Tarek Saad <[email-masked]>
Date: Wed, 23 Oct 2019 18:46:19 +0000

Hi Ebben,

Can you please advise whether you are OK with revision 11 which addresses your review comments?
https://tools.ietf.org/html/draft-ietf-mpls-base-yang-11

Regards,
Tarek

On 9/12/19, 9:18 AM, "Tarek Saad" <[email-masked]> wrote:

    Hi Ebben,
    
    Thanks for your review and comments. We have uploaded a new revision of the document that addresses the comments.
    Inline for more details.
    
    On 8/18/19, 4:19 PM, "Ebben Aries via Datatracker" <[email-masked]> wrote:
    
        Reviewer: Ebben Aries
        Review result: On the Right Track
        
        1 module in this draft:
        - [email-masked]
        
        No YANG compiler errors or warnings (pyang 2.0.1, yanglint 1.1.40, confdc 6.6.3)
        
        Module [email-masked]:
        --------------------------------------------------
        - Remove WG Chairs from contact information per
          https://tools.ietf.org/html/rfc8407#appendix-B
    [TS]: done.
    
        - 'ietf-interfaces' import should reference RFC8343 rather
    [TS]: updated.
    
        - Must clause for start/end-label is incorrect.  Would suggest moving this
          must statement underneath the `leaf end-label` as well:
        
          e.g.
        
          leaf end-label {
              type rt-types:mpls-label;
              must '. >= ../start-label' {
                  error-message
                    "The end-label must be greater than or equal " +
                    "to start-label";
              }
              description "Label-block end";
          }
    [TS]: OK, I moved this check to under end-label. I added similar check for start-label too.
    
        - Use of 'state' container under '/routing/mpls/label-blocks/label-block/state'
          These nodes could sit as r/o nodes by the looks of it directly under the
          label-block list.  In addition, do these nodes need '-count' suffixes?
          Should they rather be of type `yang:counter32`?
    [TS]: removed the 'state' container and updated to directly go under list. Also changed to type to yang:counter32.
    
          See: https://github.com/netmod-wg/FAQ/wiki/NMDA-Modelling-FAQ
        - Is there any intention to define any surrounding features?
    [TS]: updated.
        
        General comments/minor nits on the draft/modules:
        --------------------------------------------------
        - Section 1: s/feauture/feature/
        - Section 2.1: s/the the/the/
        - Section 2.1: 'labeled' vs. 'labelled'
        - Section 2.4: s/followinig/following/
        - Module line 354/367: This is for the 'active-route' action statement rather
        
     [TS]: addressed the typos, thanks.
    
    Regards,
    Tarek (for authors)