Reviewer: Jan Lindblad
Review result: Not Ready
Hi all,
This is my YANG-Doctor Last-Call review of
draft-ietf-ccamp-dwdm-if-param-yang-09. I am afraid this draft is not ready for
last call.
There are severe YANG-issues on every level here. It is quite obvious that the
authoring team has not been in the habit of compiling their YANG modules any
time recently. At first I tried to fix the issues I encountered in order to
provide helpful feedback, but after an hour I stopped. It's both too many
issues and too hard for me to guess what the authors' intent may have been.
Issue #1: Code cutoff
On page 12 shortly after revision "2016-03-17" you will find:
<CODE ENDS>
<CODE BEGINS>
This breaks the tools that extract YANG modules from RFC documents. Please
remove.
Issue #2: Yang tree
In section 4.3 of the draft, there is a YANG tree pasted. The tree
representation is often useful when a small piece of it is shown next to some
text that explains the usage and meaning of this part of the model. Here the
entire module is just pasted, which is not customary or particularly useful. I
recommend removing it, or add descriptive text for each section.
Issue #3: SNMP/MIB references
There are plenty of references to SNMP related documents, even though this
document is not related to SNMP in any way. The entire chapter 2, with the
reference to RFC 3410, for example, should probably be removed.
Issue #4: Appendix C
Appendix C contains an example, but I'm not quite sure what the example is
trying to show. I'd recommend to either explain/exemplify some more, or remove.
Issue #5: YANG compilation errors
- There are plenty of unbalanced/misplaced curly braces
list cd-penalties {}
- Indentation is generally off (which makes it harder to find the misplaced
braces)
- There are plenty of lines with missing ; at the end of the line
description "list of CD penalties"
}
- There are plenty of prefixes that do not match any imported prefix
leaf roll-off {
type layer0-types:roll-off
- A few leafs are declared to be of the non-existent type "strings" (->
"string"?)
- There are a couple of lists that names a key that does not exist
list cd-penalties {
key "penalties";
description "list of CD penalties"
}
- There is a list with no key statement at all. This may be legal, but I
suspect it's an omission in this case
- There are a number of leafs with parenthesis in the name
leaf (G.698.2) {
type strings;
- There are a few uses-statements placed inside leafs
Issue #6: Dependency on non-RFC modules
This module imports ietf-layer0-types-ext, which I would guess refers to the
work in progress defined in
https://www.ietf.org/archive/id/draft-ietf-ccamp-layer0-types-ext-01.txt If
these two documents are meant to go through the IETF processes together, this
may not be a problem.
All in all, I would think serious work remains in order to ready this draft for
last-call. The YANG module in particular seems to require a lot of work. I did
not really look closely into the YANG types, ranges, enums vs. identitites,
defaults vs. mandatory etc. that I would normally do in a LC-review. Attention
to such details will have to be paid when the module is more mature.
By the way, YANG is supposed to be written in all caps.
Best Regards,
/Jan
--
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call
