Subject: Re: [mpls] Genart last call review of draft-ietf-mpls-ldp-yang-06

From: Theresa Enghardt <[email-masked]>
Date: Sun, 5 Apr 2020 08:46:56 -0700

Hi,

Thanks for the reply and for addressing my comments.

Best,
Theresa

On 20.03.20 13:32, Kamran Raza (skraza) wrote:
> Hi Theresa,
>
> We somehow missed this email - Our apologies.
> We have updated few rev of drafts since then . The latest one being uploaded later today is rev-09 
> Please see inline [skraza20]
>
> On 2019-09-25, 3:51 AM, "Theresa Enghardt via Datatracker" <[email-masked]> wrote:
>
>     Reviewer: Theresa Enghardt
>     Review result: Ready with Issues
>     
>     I am the assigned Gen-ART reviewer for this draft. The General Area
>     Review Team (Gen-ART) reviews all IETF documents being processed
>     by the IESG for the IETF Chair.  Please treat these comments just
>     like any other last call comments.
>     
>     For more information, please see the FAQ at
>     
>     <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
>     
>     Document: draft-ietf-mpls-ldp-yang-06
>     Reviewer: Theresa Enghardt
>     Review Date: 2019-09-25
>     IETF LC End Date: 2019-10-04
>     IESG Telechat date: Not scheduled for a telechat
>     
>     Summary:
>     
>     This draft is basically ready for publication, but has some minor issues that
>     should be fixed before publication.
>     
>     Major issues: None.
>     
>     Minor issues:
>     
>     Section 1.1:
>     
>     Why is LDP IPv6 grouped in the "extended" category and not in the "base"
>     category, which the draft states to be the "minumum requirements for a typical
>     base LDP deployment" and "suffice for small deployments"? Are typical, small
>     deployments usually IPv4-only, and is this expected to remain true? Please
>     consider briefly explaining this design decision.
> [skraza20]: This was raised few times and we have clarified in emails and some edits in the doc.
>     
>     What does "igp sync" refer to? Is this the same as "igp-synchronization-delay"
>     in the extended model? Please consider expanding this abbreviation and/or
>     providing a reference.
>     
> [skraza20]: In one of prev rev, added a ref to the RFC.
>
>     Section 3:
>     
>     Could you provide references for the "widely deployed non-RFC features", which
>     are part of the extended model, please?
>     
> [skraza20]: This text has been reworked in latest rev.
>
>     "GR session is in recovery state" - What does "GR" refer to?
> [skraza20]: Graceful Restart. Expanding in rev -09.
>     
>     Section 10:
>     
>     In the Security Considerations, it would be great if you could provide some
>     examples of writable/creatable/deletable data nodes which may be considered
>     sensitive or vulnerable, and what negative effects on network operations one
>     could expect if an attacker wrote to them.
> [skraza20]: Done in rev -08.
>     
>
>     Nits/editorial comments:
>     
>     The document doesn't use any RFC 2119 keywords, yet has text resembling RFC
>     2119 boilerplate text. Please consider removing the RFC 2119 boilerplate text.
>     
> [skraza]: Already fixed in rev -07.
>
>     The document contains a few typos and grammar issues.
>     To improve readability, please check for consistency of upper/lower case terms,
>     for the use of definite and indefinite articles, and consider running a
>     spellchecker.
> [skraza20]: ack. Have fixed some known.
>     
>     Some examples:
>     
>     Section 1.1:
>     
>     "The configuration and state items are divided into following two broad
>     categories" --> "The configuration and state items are divided into the
>     following two broad categories"
>     
>     "This is worth higlighting " --> "It is worth highlighting"
>     
>     Section 3:
>     
>     "yang" - should this be all caps?
> [skraza20]: Already fixed.
>     
>     "rpc" - should this be all caps?
> [skraza20]: fixed.
>     
>     "grapically" --> "graphically"
> [skraza20]: Already fixed.
>     
>     Section 5.2.1:
>     
>     "This container falls under global tree" --> "This container falls under the
>     global tree"
>     
> [skraza20]: Fixing.
>
>     "The example of former is interface hello timers, and example of latter is
>     enabling hellos for a given AF under an interface." --> "The example of the
>     former is interface hello timers, and an example of the latter is enabling
>     hellos for a given AF under an interface."
> [skraza20]: Fixing.    
>
>     "A peer is uniquely identified using its LSR Id and hence LSR Id is the key for
>     peer list" [missing punctuation]
> [skraza20]: Already reworked.
>
>     
>     
>