Skip to main content

Last Call Review of draft-ietf-spring-sr-yang-20
review-ietf-spring-sr-yang-20-yangdoctors-lc-lhotka-2020-08-24-00

Request Review of draft-ietf-spring-sr-yang
Requested revision No specific revision (document currently at 30)
Type Last Call Review
Team YANG Doctors (yangdoctors)
Deadline 2020-08-24
Requested 2020-07-28
Requested by Joel M. Halpern
Authors Stephane Litkowski , Yingzhen Qu , Acee Lindem , Pushpasis Sarkar , Jeff Tantsura
I-D last updated 2020-08-24
Completed reviews Yangdoctors Early review of -09 by Ladislav Lhotka (diff)
Yangdoctors Last Call review of -20 by Ladislav Lhotka (diff)
Rtgdir Early review of -28 by Tal Mizrahi (diff)
Assignment Reviewer Ladislav Lhotka
State Completed
Request Last Call review on draft-ietf-spring-sr-yang by YANG Doctors Assigned
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/vYLLt0VtFQGZJ6TlOMEwSe-l-Cs
Reviewed revision 20 (document currently at 30)
Result Ready w/nits
Completed 2020-08-24
review-ietf-spring-sr-yang-20-yangdoctors-lc-lhotka-2020-08-24-00
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://datatracker.ietf.org/doc/review-ietf-spring-sr-yang-09-yangdoctors-early-lhotka-2018-10-24/

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

- The title of Section 6 (States) still looks weird to me. My suggestion is to
use "State Data" instead.

- 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.

- 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.

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