IETF Last Call Review of draft-ietf-ccamp-optical-impairment-topology-yang-18
review-ietf-ccamp-optical-impairment-topology-yang-18-yangdoctors-lc-vasko-2025-04-24-00

This review follows my previous review of an older revision of this draft. Many
of the issues I mentioned before were not addressed and so are repeated.

- all top-level configurable presence containers in the augments:
A rather odd way of configuring whether some operational information should be
reported or not but I am not an expert in this area at all. Consider using
individual features to express device capabilities or a lack of. Otherwise the
user can always filter what data to read. Also, non-presence containers may
simply not be populated if the device chooses not to do so for some reason but
it should all be documented in its description.

- leaves "roadm-path-impairments", "add-path-impairments",
"drop-path-impairments": They are leaves referencing single list instances so a
singular form should be used. Also, the last 2 are used repeatedly 4 times in
groupings used in schema level of different depth so the relative leafrefs are
different. Consider using absolute paths allowing both leaves to be put into a
grouping and be reused.

- leaf "frequency-range-id", container "frequency-range"
Nodes with duplicate definition in 3 cases of a choice, should be put into a
grouping instead.

- list "OMS-element":
The description of the list and its leaf seem to be mixed up and should be
improved. Also, the description mentions "ordered list" but the list is not
user ordered, add the YANG statement.

- leaf "is-allowed":
Use just "allowed" and the description is redundantly specific explaining that
'true' value allows the functionality and 'false' turns it off, shorten.

- redundantly long identifiers repeating information from parent nodes:
otsi-carrier-id -> carrier-id
otsi-carrier-frequency -> carrier-frequency
otsi-group-id -> id
oms-element-uid -> uid
roadm-path-impairments-id -> id
explicit-transceiver-mode-id -> id
transponder-id -> id
transceiver-id -> id
media-channel-id -> id

- identifiers with capital letters:
OMS-elements -> oms-elements
OMS-element -> oms-element

- concentratedloss:
Identifier and string used throughout the module, unless an official term,
should probably be concentrated loss or concentrated-loss.

- major text issues:
Lots of descriptions are short and not formatted as sentences, need to be
improved.

- minor text typos (full lines copied):
"It allows defining for each spectrum badwidth the
amplification  stage of the optical amplifier.";
" Deviation from the reference carrier power

- other formatting issues can be solved automatically by printing the YANG
module with yanglint or pyang.

- JSON data examples in the appendices:
The examples are not valid, use yanglint to validate all the examples and
improve them until it reports no errors or warnings.