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