Skip to main content

Last Call Review of draft-ietf-ccamp-optical-impairment-topology-yang-12

Request Review of draft-ietf-ccamp-optical-impairment-topology-yang
Requested revision No specific revision (document currently at 15)
Type Last Call Review
Team YANG Doctors (yangdoctors)
Deadline 2023-04-21
Requested 2023-03-30
Requested by Luis M. Contreras
Authors Dieter Beller , Esther Le Rouzic , Sergio Belotti , Gabriele Galimberti , Italo Busi
I-D last updated 2023-04-04
Completed reviews Yangdoctors Last Call review of -12 by Michal Vaško (diff)
Clustered with draft-ietf-ccamp-rfc9093-bis
Assignment Reviewer Michal Vaško
State Completed
Request Last Call review on draft-ietf-ccamp-optical-impairment-topology-yang by YANG Doctors Assigned
Posted at
Reviewed revision 12 (document currently at 15)
Result On the Right Track
Completed 2023-04-04
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

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