Skip to main content

Last Call Review of draft-ietf-ccamp-l1csm-yang-15
review-ietf-ccamp-l1csm-yang-15-yangdoctors-lc-clarke-2021-11-29-00

Request Review of draft-ietf-ccamp-l1csm-yang-15
Requested revision 15 (document currently at 26)
Type Last Call Review
Team YANG Doctors (yangdoctors)
Deadline 2021-12-23
Requested 2021-11-25
Requested by Daniele Ceccarelli
Authors Young Lee , Kwang-koog Lee , Haomian Zheng , Oscar Gonzalez de Dios , Daniele Ceccarelli
I-D last updated 2021-11-29
Completed reviews Intdir Telechat review of -25 by Antoine Fressancourt (diff)
Genart Last Call review of -24 by Dan Romascanu (diff)
Secdir Last Call review of -24 by Yaron Sheffer (diff)
Yangdoctors Early review of -07 by Robert Wilton (diff)
Yangdoctors Last Call review of -15 by Joe Clarke (diff)
Rtgdir Last Call review of -19 by Adrian Farrel (diff)
Rtgdir Last Call review of -19 by Nicolai Leymann (diff)
Assignment Reviewer Joe Clarke
State Completed
Request Last Call review on draft-ietf-ccamp-l1csm-yang by YANG Doctors Assigned
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/XOZzJsYbV0Svo0W_a7g6kz5isDk
Reviewed revision 15 (document currently at 26)
Result Ready w/nits
Completed 2021-11-29
review-ietf-ccamp-l1csm-yang-15-yangdoctors-lc-clarke-2021-11-29-00
I have been asked to review this draft and the L1CSM YANG module it includes on
behalf of YANG Doctors.  Overall, I think the draft/module is ready, but I did
find a few small issues.  First, please run "pyang -f yang" on this to
normalize its formatting.  There were a few spacing and other formatting issues
that will be cleaned up by doing this.  Other items are below:

In the header of the YANG module, you import ietf-layer1-types with a mark of
RFCYYYY.  In your comment below calling out RFCXXXX (i.e., this RFC) it would
be good to point the RFC Editor to YYYY as well.

===

In your identity definition section, you have a number of identity descriptions
which are self-referential.  Admittedly, this is one of my pet peeves, but if
you could add more clarifying description text to identities such as
one-way-delay, one-way-errored-second, etc. it would help implementors and
operators better consume the YANG module without always needing to refer to
external references.

===

Can time-interval be uint32 instead of int?  I do not thing a negative time
interval is possible/useful here.

===

The description of the grouping subscriber-l1vc-sls-service-attributes doesn't
look accurate.  This is not a value per se, but a collection of attributes.

===

In the /services/service list you explicitly have "mandatory true" for the key
service-id.  This is not needed since the key is mandatory.