[mpls] Yangdoctors last call review of draft-ietf-mpls-msd-yang-05

Jan Lindblad via Datatracker <noreply@ietf.org> Tue, 28 May 2024 14:00 UTC

MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Jan Lindblad via Datatracker <noreply@ietf.org>
To: yang-doctors@ietf.org
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <171690483065.41857.12285021065441747282@ietfa.amsl.com>
Date: Tue, 28 May 2024 07:00:30 -0700
Message-ID-Hash: 2FQSQXVR3AWKVOOTISX3FSM3MIDGTGCU
CC: draft-ietf-mpls-msd-yang.all@ietf.org, last-call@ietf.org, mpls@ietf.org
Reply-To: Jan Lindblad <jlindbla@cisco.com>
Subject: [mpls] Yangdoctors last call review of draft-ietf-mpls-msd-yang-05
Archived-At: <https://mailarchive.ietf.org/arch/msg/mpls/CjzGWTMLi0uylAcItlOAO2V9y7Y>

Reviewer: Jan Lindblad
Review result: Ready with Nits

Hi all,

This is my YANG-Doctor Last-Call review of draft-ietf-mpls-msd-yang-05. I
previously reviewed the -02 version. This draft contains two small and focused
modules. They were in pretty good shape already in -02, and the few nits I had
then have all been fixed.  Still, I did notice a couple of nits to look at.

Nit #1: Tree diagram not updated

Section 3 of the draft contains this YANG tree:

   module: ietf-mpls-msd
     augment /rt:routing/mpls:mpls:
       +--ro node-msd
          +--ro node-msds* [msd-type]
             +--ro msd-type?    identityref
             +--ro msd-value?   uint8
     augment /rt:routing/mpls:mpls/mpls:interfaces/mpls:interface:
       +--ro link-msd
          +--ro link-msds* [msd-type]
             +--ro msd-type?    identityref
             +--ro msd-value?   uint8

The msd-type and msd-value leafs are shown with a "?"-suffix, indicating that
they are optional. They were back in -02, but as I advised, they have been made
mandatory in -05. Great. Just need to update the tree diagram to reflect the
new situation (regenerate the tree diagram).

Nit #2: Plural container, singular list

As can be seen in the above tree diagram, there is a container "node-msd" with
a list "node-msds" inside, and similarly for "link-msd" and "link-msds". This
matches the YANG container+list convention very well, except the plural form is
normally on the outer/container level and the singular is on the list.

Even if this makes no functional difference, for the sake of the principle of
least surprise, I would suggest renaming the containers and lists as follows:

    container node-msds {
      config false;
      description
        "Maximum SID Depth (MSD) of a node.";
      list node-msd {

    container link-msds {
      config false;
      description
        "Maximum SID Depth (MSD) of an interface.";
      list link-msd {

Best Regards,
/jan

[mpls] Yangdoctors last call review of draft-ietf… Jan Lindblad via Datatracker
[mpls] Re: Yangdoctors last call review of draft-… Yingzhen Qu
[mpls] Re: Yangdoctors last call review of draft-… Jan Lindblad (jlindbla)