Document: draft-ietf-ccamp-optical-impairment-topology-yang Title: A YANG Data Model for Optical Impairment-aware Topology Reviewer: Michal Vaško Review result: On the Right Track 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. -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
