Hi Yingzhen,

I do not understand why you want to keep the 2 nodes in `ietf-isis-link-attr` the way they are. Whether configuration or state data, by using statements such as `range` you can formally and exactly specify the allowed value space, why would you not want to use it? If you put this information in the `description`, it is understandable only by a human. Also, why not be consistent and use the `range` for one node but not the other. Finally, duplicating information is always redundant.

In any case, I am only providing YANG-specific advice which you are essentially free not to follow but at least another opinion of a YANG doctor may be helpful.

Regards,
Michal

On 18. 9. 2025 0:24, Yingzhen Qu wrote:
Hi Michal,

Thanks for the review. I've updated the JSON example in the appendix. However, I don't think those nits in ietf-isis-flex-alog need to be fixed. Details below inline.

Thanks,
Yingzhen

On Wed, Sep 17, 2025 at 1:06 AM Michal Vasko <[email protected]> wrote:

    Hi Yingzhen,

    it seems the minor issues in `ietf-isis-link-attr` were fixed but
    the ones in `ietf-isis-flex-algo` were not:

    ietf-isis-flex-algo
    - leaf flex-algo - range in description instead of using the
    "range" statement

[Yingzhen]: This is read only data. The description is only for information.

    - leaf algo-number - redundant range mentioned in the description

[Yingzhen]: I don't consider this as a nit.

    Also, the XML data example now validates but the JSON one seems to
    be using syntax rules unknown to me and definitely not the ones in
    RFC 7951. I have attached a valid YANG data JSON example.

[Yingzhen]: I've updated the JSON example.

    Regards,
    Michal

    On 12. 9. 2025 8:13, Yingzhen Qu wrote:
    Hi Michal,

    Thanks for the review. I have uploaded a new version to address
    your comments.

    I fixed all the editorial issues in your comments and updated the
    examples. Please let me know if you have any other questions or
    comments.

    Thanks,
    Yingzhen

    On Fri, Aug 29, 2025 at 12:17 AM Michal Vaško via Datatracker
    <[email protected]> wrote:

        Document: draft-ietf-lsr-isis-flex-algo-yang
        Title: YANG Model for IS-IS Application-Specific Link
        Attributes and Flexible
        Algorithm Reviewer: Michal Vaško Review result: Ready with Issues

        Looking at the 2 YANG modules, they are in a good shape and I
        found only a few
        nits (without having knowledge of the ISIS routing protocol).
        However, the XML
        and JSON example YANG data are invalid, which needs to be
        addressed.

        ietf-isis-link-attr
        - leaf udabm-length - 2nd line of description has extra space
        - leaf transition - 2nd line of description has extra space
        - leaf unidirectional-link-delay - units mentioned in
        description instead of
        "units" statement - uses
        application-specific-link-attributes-sub-tlv - second
        usage has extra indent

        ietf-isis-flex-algo
        - leaf flex-algo - range in description instead of using the
        "range" statement
        - leaf algo-number - redundant range mentioned in the description

        Appendix A invalid (yanglint used):
        - none of the XML prefixes of the used identities are defined
        - there are leafrefs to the ietf-te module data, which should
        ideally be
        included so that the examples can successfully be validated
        or at least the
        used ietf-te data referenced, if found in another RFC - "At
        least one
        area-address must be configured."
        
(/ietf-routing:routing/control-plane-protocols/control-plane-protocol[type='ietf-isis:isis'][name='default']/ietf-isis:isis)

        Appendix B invalid:
        - some of the prefixes used are not valid module names
        (iana-metric-type and
        iana-algo-types) - similar leafref problems - similar
        explicit validation error
        message


Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

_______________________________________________
Lsr mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to