Yingzhen,
I had a look at the
draft-ietf-mpls-msd-yang-07 now, and I think the two YANG modules within are *ready* from a YANG Doctor point of view. Well done!
Best Regards,
/jan
From: Yingzhen Qu <yingzhen.ietf@xxxxxxxxx>
Date: Tuesday, 28 May 2024 at 20:18
To: Jan Lindblad (jlindbla) <jlindbla@xxxxxxxxx>
Cc: yang-doctors@xxxxxxxx <yang-doctors@xxxxxxxx>, draft-ietf-mpls-msd-yang.all@xxxxxxxx <draft-ietf-mpls-msd-yang.all@xxxxxxxx>, last-call@xxxxxxxx <last-call@xxxxxxxx>, mpls@xxxxxxxx <mpls@xxxxxxxx>
Subject: Re: Yangdoctors last call review of draft-ietf-mpls-msd-yang-05
Hi Jan,
Thanks for the review and comments. I've uploaded version -06 and fixed the nits. Please kindly let us know if you see any other issues.
On Tue, May 28, 2024 at 7:00 AM Jan Lindblad via Datatracker <noreply@xxxxxxxx>
wrote:
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
|
--
last-call mailing list -- last-call@xxxxxxxx
To unsubscribe send an email to last-call-leave@xxxxxxxx
