Subject: Re: [Rift] Yangdoctors last call review of draft-ietf-rift-yang-03

From: <[email-masked]>
Date: Thu, 9 Sep 2021 17:09:26 +0800 (CST)

Hi Michal, 
Thank you again for your review! 
We are do the updating work. 
But we are not sure that we got the meaning of the following comment:
"algorithm-type - empty cases - redundant on their own, are expected to
be augmented?"
The algorithm-type leaf in the model is not empty. Could you please explain this comment more detailedly?
Thank you very much!
Best regards,
Sandy

------------------原始邮件------------------
发件人：MichalVaškoviaDatatracker
收件人：[email-masked];
抄送人：[email-masked];[email-masked];[email-masked];
日 期 ：2021年07月08日 18:17
主 题 ：[Rift] Yangdoctors last call review of draft-ietf-rift-yang-03
Reviewer: Michal Vaško
Review result: Almost Ready

Generally, use references where make sense (features, nodes) and use units
and/or standard types (ietf-yang-types) for leaves (such as grouping
neighbor-node/bandwidth). All links are invalid, better to use references
anyway because the module will be used outside the RFC.

Specific problems:

- description - copyright 2020
- typedef ieee802-1as-timestamp-type - reference in description, put separately
- grouping address-families
- list with a single key can be leaf-list
- would make sense if meant to be augmented with new nodes
- grouping node-flag
- used only once, makes sense if meant to be reused by other modules
- consider using bits type in the leaf
- grouping base-node-info/pod - redundant description, use union of number and
"undefined", or leave out for undefined since it is not mandatory - augment
rift/rx-lie-multicast-address,tx-lie-multicast-address
- default value in description - should be defined in YANG
- consider using
refine on
"addresses"
rx-flood-port - redundant default in description, is obvious in YANG
algorithm-type - empty cases - redundant on their own, are expected to
be augmented? HAL - use lowercase
database/tie/negative_disaggregation_prefixes
- use hyphen instead of underscore
- consider abbreviated/shorter node names
- applicable for the following nodes as well

_______________________________________________
RIFT mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/rift

---

From: Michal Vaško <[email-masked]>
Date: Thu, 09 Sep 2021 11:26:37 +0200

Hi Sandy,
firstly, sorry for the chaotic review. I have organized it properly but some formatting was lost, I will know next time.

As for your question, I do not see any node "algorithm-type" of type leaf, only choice. This choice includes 2 empty cases "spf" and "all-path". This YANG construct as it is has no purpose and cannot even be instantiated in YANG data, unless some YANG data nodes were added into the cases by augments. I suppose that is not the intention and you simply wanted to write a configuration node that could be set to either "spf" or "all-path" value. In that case you want to use enumeration like this:

leaf algorithm-type {
  type enumeration {
    enum spf {
      description "The algorithm is SPF.";
    }
    enum all-path {
      description "The algorithm is all-path.";
    }
  }
  description "The possible algorithm types.";
}

If you need the possible "algorithm-type" values to be dynamic - allowed new to be added from other YANG modules, you can even use type "identityref" and define the values as identities.

Regards,
Michal

---

From: <[email-masked]>
Date: Thu, 9 Sep 2021 17:40:38 +0800 (CST)

Hi Michal, 
Got it. We'll change the type of the leaf to enum.
Thank you very much!
Best regards,
Sandy

------------------原始邮件------------------
发件人：MichalVaško
收件人：张征00007940;
抄送人：[email-masked];[email-masked];[email-masked];[email-masked];[email-masked];
日 期 ：2021年09月09日 17:27
主 题 ：Re: [yang-doctors] [Rift] Yangdoctors last call review of draft-ietf-rift-yang-03
Hi Sandy,
firstly, sorry for the chaotic review. I have organized it properly but some formatting was lost, I will know next time.
As for your question, I do not see any node "algorithm-type" of type leaf, only choice. This choice includes 2 empty cases "spf" and "all-path". This YANG construct as it is has no purpose and cannot even be instantiated in YANG data, unless some YANG data nodes were added into the cases by augments. I suppose that is not the intention and you simply wanted to write a configuration node that could be set to either "spf" or "all-path" value. In that case you want to use enumeration like this:
leaf algorithm-type {
type enumeration {
enum spf {
description "The algorithm is SPF.";
}
enum all-path {
description "The algorithm is all-path.";
}
}
description "The possible algorithm types.";
}
If you need the possible "algorithm-type" values to be dynamic - allowed new to be added from other YANG modules, you can even use type "identityref" and define the values as identities.
Regards,
Michal