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

Hi Kamran,

Thanks for the reply. Looking forward to the next rev.

Happy Holidays!

Thanks,
Yingzhen

On Mon, Dec 23, 2019 at 10:10 AM Kamran Raza (skraza) <skraza@cisco.com>
wrote:

> Hi Yingzhen,
>
> Thanks for your detailed review. We are addressing your comments as well
> as YD comments and plan to spin a new rev in new year.
> Please see our response inline - tagged as [skraza]
>
> On 2019-11-01, 5:25 PM, "mpls on behalf of Yingzhen Qu via Datatracker" <
> mpls-bounces@ietf.org on behalf of noreply@ietf.org> wrote:
>
>     Reviewer: Yingzhen Qu
>     Review result: Has Issues
>
>     I have been selected as the Routing Directorate reviewer for this
> draft. The
>     Routing Directorate seeks to review all routing or routing-related
> drafts as
>     they pass through IETF last call and IESG review, and sometimes on
> special
>     request. The purpose of the review is to provide assistance to the
> Routing ADs.
>     For more information about the Routing Directorate, please see
>     http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
>
>     Although these comments are primarily for the use of the Routing ADs,
> it would
>     be helpful if you could consider them along with any other IETF Last
> Call
>     comments that you receive, and strive to resolve them through
> discussion or by
>     updating the draft.
>
>     Document: draft-ietf-mpls-ldp-yang
>     Reviewer: Yingzhen Qu
>     Review Date: Nov 1st, 2019
>     Intended Status: Standards Track
>
>     Summary:
>
>     This document is near ready for publication. It has some issues that
> should be
>     at least considered prior to publication.
>
>     Comments:
>
>     Thanks for working on this draft. As an active YANG contributor I
> appreciate
>     the work here.
>
>     Major issues:
>
>     I’m not sure whether this should be considered as a major issue, which
> is how
>     the document is structured. The draft separates the configuration and
> operation
>     states into two sections, and this seems to be a before NMDA product
> and a bit
>     redundant to me. The tree structures are used in different ways
> multiple times
>     in the document, and this significantly reduces the readability of the
> modules.
>
> [skraza]: Ack. Will fix.
>
>     Minor Issues:
>
>     I feel the English in this draft could be improved, but I’m not a
> native
>     speaker. Maybe RFC editor will help with this?
>
>     It seems that “model” and “module” are used exchangeable in this
> document,
>     please make them consistent.
>
> [skraza]: Ack. Will fix.
>
>     “YANG” should be capital cased, and there are “yang” at multiple
> places in this
>     document. Please fix them.
>
> [skraza]: Ack. Will fix.
>
>     Please consider add names to figures.
>
> [skraza]: Ack. Will fix.
>
>     I understand inheritance is an important feature of document. I’d
> suggest maybe
>     add a section/paragraph in “overview” to introduce the concept and how
> it works
>     instead of repeating the idea with examples in every major container
> design.
>
>
>  [skraza]: Ack. Will see how to address this.
>
>     Nits for your consideration:
> [skraza]: Ack to all the below nits - will fix.
>
> Rgds-
> --
> Kamran (on behalf of authors)
>
>     Section 1.1
>     Whereas, the "extended" category contains all other non-base features.
>     Please consider remove “all” because not all other features are
> covered.
>
>     Section 3
>     extended "ietf-mpls-ldp-extended" module that models the extended
>           LDP features and augments the base LDP
>     Please consider removed “extended” at the beginning, and add “module”
> at the
>     end, “augments the base LDP module”.
>
>     There are four main containers in our module(s):
>     I suppose you meant “four types of containers”?
>
>     Section 4
>     under LDP base and extended.
>     Please add a “module” at the end.
>
>     Section 5
>     Following is the high-level configuration organization for base LDP:
>     Please add a “module” at the end.
>
>     Typo in figure 3 “targeteted”
>
>     Typo in figure 4 “targeteted”
>
>     Given the configuration hierarchy, the model allows inheritance such
>        that an item in a child tree is able to derive value from a similar
>        or related item in one of the parent.
>     for grammar, it should be “one of the parents”, but this sentence is a
> bit
>     confusing. I’d suggest add a bit more explanation how inheritance work.
>
>     5.1.1
>     Missing period “.” at the end of the first sentence.
>
>     The tree showing here is not a complete tree, just want to make sure
> whether
>     this is intentional?
>
>     5.1.2
>     Missing period “.” at the end of the first sentence.
>
>     5.2.1.1
>     “LSR id” and “LSR Id” both are used here, please keep them consistent.
>
>     5.2.1.5
>     Missing “.” at the end of the first paragraph.
>
>     “A peer is uniquely identified using its
>      LSR Id and hence LSR Id is the key for peer list”
>     Please consider simplify the sentence to “A peer is uniquely
> identified by its
>     LSR Id.”
>
>     Section 6
>     “  Operational state of LDP can be queried and obtained from read-only
>        state containers that fall under the same tree (/rt:routing/
>        rt:control-plane-protocols/) as the configuration.”
>     This sentence is a bit confusing. Please consider revise it.
>
>     _______________________________________________
>     mpls mailing list
>     mpls@ietf.org
>     https://www.ietf.org/mailman/listinfo/mpls
>
>
>