Re: [Last-Call] Yangdoctors last call review of draft-ietf-spring-sr-yang-20

Yingzhen Qu <yingzhen.qu@xxxxxxxxxxxxx> · Tue, 25 Aug 2020 20:16:12 +0000

Hi Lada,

Thank you for your review, really appreciate. 

I've uploaded version -21 to address your comments and please see detailed answers below inline starting with [YQ].

Thanks,
Yingzhen

On 8/24/20, 5:51 AM, "Ladislav Lhotka via Datatracker" <noreply@xxxxxxxx> wrote:

    Reviewer: Ladislav Lhotka
    Review result: Ready with Nits

    I also did an early YANG Doctors review [1]. My comments regarding YANG module
    revisions and normative references are addressed in the current revision. The
    suggested naming changes were either accepted or, I assume, addressed in the WG
    and rejected (which is OK).

    Compared to the previously reviewed revision -09, the current revision contains
    one additional YANG module: ietf-segment-routing-mpls. This module adheres to
    the same high standards as the previous two, and I discovered no issues with
    all of them.

    [1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdatatracker.ietf.org%2Fdoc%2Freview-ietf-spring-sr-yang-09-yangdoctors-early-lhotka-2018-10-24%2F&amp;data=02%7C01%7Cyingzhen.qu%40futurewei.com%7Cb6171db579ff4df2c02e08d8482c6cde%7C0fee8ff2a3b240189c753a1d5591fedc%7C1%7C0%7C637338702924404727&amp;sdata=%2BCVcN8RQQ%2B2aDP%2BFHY2GiCukPEzpf1GcJAtQ0LDbBh4%3D&amp;reserved=0

     Comments
    ------------

    - The title of Section 6 (States) still looks weird to me. My suggestion is to
    use "State Data" instead.
[YQ]: modified as you suggested.

    - The title of Section 8 should use plural "YANG Modules" because it contains
    three modules. It would also be helpful to introduce a subsection for each
    module.
[YQ]: changed the title and added an introduction of each module.

    - Due to the RFC line length limit, the example in Appendix A uses a line break
    inside a URI of a XML namespace declaration, which makes the XML invalid. This
    can be probably avoided by including the XML namespace declaration for "sr-cmn"
    in the top-level element, i.e.

      <routing
        xmlns="urn:ietf:params:xml:ns:yang:ietf-routing"
        xmlns:sr-cmn="urn:ietf:params:xml:ns:yang:ietf-segment-routing-common">

      If not, it would be better to use conventions of RFC 8792.

[YQ]: I tried the format as your suggested, but somehow I could get it pass yanglint, so added "\" per RFC 8792.

    - Assuming that the example is intended for human readers, it might be better
    to provide it in the JSON representation per RFC 7951.

[YQ]: Added JSON format.

-- 
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call