Skip to main content

Early Review of draft-ietf-teas-yang-te-mpls-03
review-ietf-teas-yang-te-mpls-03-yangdoctors-early-moberg-2020-09-16-00

Request Review of draft-ietf-teas-yang-te-mpls
Requested revision No specific revision (document currently at 04)
Type Early Review
Team YANG Doctors (yangdoctors)
Deadline 2020-08-31
Requested 2020-07-30
Requested by Lou Berger
Authors Tarek Saad , Rakesh Gandhi , Xufeng Liu , Vishnu Pavan Beeram , Igor Bryskin
I-D last updated 2020-09-16
Completed reviews Yangdoctors Early review of -03 by Carl Moberg (diff)
Comments
This is an early review in preparation for WG LC
Assignment Reviewer Carl Moberg
State Completed
Request Early review on draft-ietf-teas-yang-te-mpls by YANG Doctors Assigned
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/n3W3mxYus6v9L-ctjGY2Jvt_3k0
Reviewed revision 03 (document currently at 04)
Result Almost ready
Completed 2020-09-16
review-ietf-teas-yang-te-mpls-03-yangdoctors-early-moberg-2020-09-16-00
This is my (belated, apologies) YANG Doctors early review of the ietf-te-mpls
module in draft-ietf-teas-yang-te-mpls-03.

The YANG module extracts and passes validation cleanly, and seems well designed
for its intended purpose. The authors have done a good job. My comments in this
review is focused on suggestions on improving readability and general
uniformity.

I would suggest running the YANG module through the YANG output format of the
pyang tool with the canonical option, i.e.:
 $ pyang -f yang --yang-canonical ietf-te-mpls@2020-03-09.yang

While this will introduce a few spots of whitespace weirdness in the
description fields that will need to be fixed it will generally line up the
formatting consistently across statements, line breaks and other places.

I also believe the description fields could do with some formatting attention.
I am mostly thinking of capitalization and punctuation which varies across.

Apart from that, a few smaller suggestions:

There is a pair of groupings that are used in subsequent groupings for later
reuse. - grouping tunnel-igp-shortcut-config used in grouping
tunnel-igp-shortcuts - grouping tunnel-forwarding-adjacency-configs used in
grouping tunnel-forwarding-adjacency I would suggest using 'config' (singular)
or 'configs' (plural) consistently.

Is there a specific reason to introduce underscores in the identifier string in
the 'tunnel-bandwidth_top' and 'te-path-bandwidth_top' groupings?

There is what seems to be a stray comment on line 413 in the extracted YANG:
  /* MPLS TE interface augmentations */

The next to last augment statement has a description that says "foo".