Reviewer: Michal Vaško Review result: On the Right Track The reviewed YANG module should describe the parameters it intends to describe but there are lots of formal shortcomings in YANG formatting and general consistency. To fix formatting, pyang tool can be used. Description texts should be full sentences. Data examples are not valid (even include a note that they need to be updated), use yanglint to validate them against the YANG module. Other than that, some specific improvements: ## line 170: leaf out-voa - units dB seems redundant, it is part of the used type name ## 326 - 379: grouping roadm-add-path ## 426 - 478: grouping roadm-drop-path - all the 5 leaves seems to match exactly those in the grouping roadm-express-path above so I would just put uses in this grouping ## 405, 571: leaf roadm-noise-figure - lots of the leaves are using types from l0-types - a local typedef can be defined (if no existing one can be used) that would prevent duplication of all the information and move type specifics out of the data nodes - alternatively a grouping with this leaf can be defined to avoid description duplication but that is up to you, seems unnecessary ## 631: leaf nominal-power-spectral-density - similar problem as the previous one except a grouping makes no sense, just a typedef ## 287, 298, 635, 686: - union with an empty type is used a lot and selecting this type is usually explained but not in these cases ## 880: augment "/nw:networks/nw:network/nw:network-types/tet:te-topology" - adding an empty presence container, which does not make sense on its own unless other foreign augments are expected - following 2 augments add into this container, why not merge all 3 augments into one to prevent any confusion? ## 899: container otsi-information - including "information" in the identifier seems redundant ## 1293, 1315, 1335, 1359, 1412, 1468: leaf roadm-path-impairments - all the leaves are "config false" when augments are applied but some have it specified explicitly some not - if the leafref path is changed to an absolute path, the leaf can be in a grouping and used at all the places ## 1492 - 1509; 1524 - 1541; 1570 - 1579; 1614 - 1623: - both leaves add-path-impairments and drop-path-impairments use the same absolute path leafref so a single typedef can be defined (also used for the leafref in the leaf roadm-path-impairments above) - then both leaves can be put into a grouping and used at all 4 places Regards, Michal -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
