> On Dec 16, 2020, at 7:12 AM, Ladislav Lhotka via Datatracker 
> <[email protected]> wrote:
> 
> Reviewer: Ladislav Lhotka
> Review result: Ready with Issues
> 
> **** General comments
> 
> - YANG module "ietf-isis-reverse-metric" is in a good shape except for
>  one issue described below.
> - I appreciate the three examples of instance data in appendices (two
>  in XML representation, one in JSON).
> 
> **** Specific comments
> 
> - In three places, the module uses "when" expressions with plain
>  string equality test applied to identityref values, such as:
> 
>     when "../rt:type = 'isis:isis'"
> 
>  This is known to be fragile, which can demonstrated on the JSON
>  example in appendix A.3: It has
> 
>     "type": "ietf-isis:isis"
> 
>  which makes the "when" expression false because of the different
>  prefix, and the corresponding augment doesn't happen.
> 
>  The above "when" expression should use the XPath function
>  "derived-from-or-self" that is defined in RFC 7950:
> 
>     when "derived-from-or-self(../rt:type, 'isis:isis')"

Will change.

> 
> - The "tlv16-reverse-metric" grouping is used only once in the
>  module. Unless it is expected to be used elsewhere, I would
>  recommend to remove this grouping and use its contents directly.

As this was describing TLV data I figured it might be useful to other modules 
that work with TLVs.

> 
> - The phrase "YANG XML data" is somewhat misleading, although it is
>  probably often used in informal discussions. I would suggest "XML
>  instance data" instead.

Updated.


> 
> **** Nits
> 
> - Abstract & Introduction: s/routeing/routing/

I'm actually taking that from the standard which names the protocol. :)

> 
> - Appending A.3: s/YANG XML data/JSON instance data/

Fixed.

Thanks,
Chris.

> 
> 
> 
> --
> last-call mailing list
> [email protected]
> https://www.ietf.org/mailman/listinfo/last-call
> 

Attachment: signature.asc
Description: Message signed with OpenPGP

_______________________________________________
Lsr mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/lsr

Reply via email to