Early Review of draft-ietf-teas-yang-path-computation-11
review-ietf-teas-yang-path-computation-11-yangdoctors-early-bjorklund-2020-12-17-00
| Request | Review of | draft-ietf-teas-yang-path-computation-10 |
|---|---|---|
| Requested revision | 10 (document currently at 18) | |
| Type | Early Review | |
| Team | YANG Doctors (yangdoctors) | |
| Deadline | 2020-12-18 | |
| Requested | 2020-11-14 | |
| Requested by | Vishnu Pavan Beeram | |
| Authors | Italo Busi , Sergio Belotti , Oscar Gonzalez de Dios , Anurag Sharma , Daniele Ceccarelli | |
| Draft last updated | 2020-12-17 | |
| Completed reviews |
Yangdoctors Early review of -11
by
Martin Björklund
(diff)
|
|
| Assignment | Reviewer | Martin Björklund |
| State | Completed | |
| Review |
review-ietf-teas-yang-path-computation-11-yangdoctors-early-bjorklund-2020-12-17
|
|
| Posted at | https://mailarchive.ietf.org/arch/msg/yang-doctors/3aMcbAsaiCx3r7tAOgXjmtGffy4 | |
| Reviewed revision | 11 (document currently at 18) | |
| Result | Ready with Nits | |
| Completed | 2020-12-17 |
review-ietf-teas-yang-path-computation-11-yangdoctors-early-bjorklund-2020-12-17-00
o General
The language is called "YANG", not "Yang".
o 1.2. Tree Diagram
The text says:
A simplified graphical representation of the data model is used in
section 6.1 of this this document. The meaning of the symbols in
these diagrams is defined in [RFC8340].
Tree diagrams are used also in chapter 5. I suggest:
Tree diagrams used in this document follow the notation defined in
[RFC8340].
o Tree diagrams in general
You can use pyang -f tree --tree-line-length 68 ... in order to
avoid long lines in the RFC.
o 6.1
This section presents a fully expanded tree diagram of the module.
Tree diagrams are mainly used to give an overview of a module's
structure. The tree diagram in this section spans 13 pages and is
quite hard to read.
I also note that a majority of the nodes in this tree diagram come
from the expansion of groupings that aren't defined in this
document. Hence, I suggest that you might want to run:
pyang -f tree --tree-line-length 68 \
--tree-print-groupings --tree-no-expand-uses
o There are a number of groupings that are used only once, and do not
seem to be defined to be reused by other modules, e.g.,
"requested-info", "requested-state", "svec-metrics-bounds" and more.
If they are intended to be reused, it should be made clear in their
description statements. If not, I think they should be inlined and
removed.
o grouping svec-exclude
This grouping has an ordered-by user list. Why is this list user
ordered? If the order matters, it should be explained how it matters.
Also, the index leaf has this description:
"XRO subobject index"
What is "XRO"? Is this description sufficiently clear?
o path-request
In the path-request, there is construct for path-refs:
list primary-reverse-path-ref {
key index;
min-elements 1;
description
"The list of primary reverse paths that
reference this path as a candidate
secondary reverse path";
leaf index {
type uint32;
description
"The index used by the
primary-reverse-path-ref list";
}
What is this index? Is it only used as an arbitrary index, or
something else? If it is an arbitraty index, it should be explained
in the descriptions.
Also note that lists in rpc input don't need an index.
o Validation
The module fails YANG validation, but that is really due to errors
in ietf-te@2020-07-12.yang. Specifially, the leafref in the
grouping "path-compute-info" must have prefixes in its path.
Without prefixes, the path refers to nodes in the module that uses
the grouping. (same for other groupings in that module).
o Layout
I suggest you run the module through
pyang -f yang --yang-canonical --yang-line-length 68
in order to have the module indented and formatted consistently.
This will make the RFC editor's job easier.
/martin