Re: [Teas] Yangdoctors early review of draft-ietf-teas-yang-path-computation-11

Hi Tom,

Yes, we have discussed this issue with the authors of ietf-te and the fix has been published in the -26 draft revision

Now the IETF datatracker reports no errors/warnings for the path-computation draft

Thanks to you and Martin for spotting it

Italo

> -----Original Message-----
> From: tom petch [mailto:ietfa@btconnect.com]
> Sent: mercoledì 24 febbraio 2021 11:24
> To: yang-doctors@ietf.org; Martin Björklund <mbj+ietf@4668.se>
> Cc: draft-ietf-teas-yang-path-computation.all@ietf.org; teas@ietf.org
> Subject: Re: [Teas] Yangdoctors early review of draft-ietf-teas-yang-path-
> computation-11
> 
> From: Teas <teas-bounces@ietf.org> on behalf of Martin Björklund via
> Datatracker <noreply@ietf.org>
> Sent: 17 December 2020 18:58
> 
> Reviewer: Martin Björklund
> Review result: Ready with Nits
> 
> <tp>
> 
> Martin's review flagged an error in the ietf-te module whereby a path
> statement lacked prefixes causing validation to fail  That module is updated in
> teas-yang-te-26 to add those missing prefix which may overcome  that issue.
> 
> Tom Petch
> 
> 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
> 
> 
> 
> _______________________________________________
> Teas mailing list
> Teas@ietf.org
> https://www.ietf.org/mailman/listinfo/teas