[Trimming the list to open issues] Hi Yingzhen,
> On May 23, 2020, at 1:21 PM, Yingzhen Qu <[email protected]> wrote: > > Hi Mahesh, > > Thanks for the review, very much appreciated. > > I submitted -10 version to address your comments: > https://datatracker.ietf.org/doc/draft-ietf-rtgwg-policy-model/ > <https://datatracker.ietf.org/doc/draft-ietf-rtgwg-policy-model/> > > Note: There are YANG validation errors because yanglint can't find model > "ietf-if-extensions", not sure why. > yanglint SO 1.6.7: yanglint --verbose -p {rfclib} -p {draftlib} -p {tmplib} > {model} -i: > err : Data model "ietf-if-extensions" not found. > err : Importing "ietf-if-extensions" module into "ietf-routing-policy" failed. > err : Module "ietf-routing-policy" parsing failed. ietf-if-extensions is part of the draft-ietf-netmod-inft-ext-yang draft, which is still a draft. It should ideally be in the {draftlib} path variable. I would suggest a open an issue with the tools team to have them track it. I am assuming of course that you are able to validate the model locally on your machine. … > The document includes a section on Terminology and Notation. However it > does > not define what is a "routing policy" or provide a reference to it. > [YQ]: I'm not sure there is a standard definition of "routing policy". I'll > do some > research on this and see whether we should add it to the terminology. Looking at how the draft describes it, how about something like this? “A routing policy defines whether a routing protocol imports a route into the routing table, modifies it, or exports a route from the routing table." … > Section 10.1 - Routing policy model > > The model uses a lot of groupings, most of them used only once in the > model. > That makes the model difficult to understand. Unless these groupings are > meant > for use by other models, they should be folded into the main container. > Take > this grouping for example: > > grouping policy-definitions { > description > "This grouping provides policy definitions"; > > leaf name { > type string; > description > "Name of the top-level policy definition -- this name > is used in references to the current policy"; > } > } > > For one the description of the grouping is not correct. Secondly, it is > used > only once in the model and that too to define a key of a list. > > [YQ]: This model was initiated by OpenConfig, we've removed some groupings > Since this become an IETF draft. I removed "policy-definitions" and > "policy-statements" > groups. The general rule is that if the grouping is used only once in the model, and not meant for use outside the module, that it should not be a grouping. For example, ‘grouping neighbor-set’ is used only once in the model, and I doubt it is meant for use outside of this module. That grouping should be collapsed into the model where it gets used. Same for ‘grouping tag-set’. These are just two examples. There are many more. > > The YANG model imports multiple models from other drafts. It provides > normative > references to the imported models in Section 2.2. (A nit here is that it > missing the cross-reference in the table for if-l3-vlan), A nit. if-l3-vlan is still missing the cross-reference (and the hyperlink) to the draft. Also why idnits is giving you an error at the bottom. > but does not provide > references to them in the model. See Section 3.9 of RFC 8407. Please add > something like this in the model. > > import ietf-inet-types { > prefix "inet"; > reference > "RFC 6991: Common YANG Data Types."; > } > [YQ]: fixed. > > Please drop <mailto:....> and just keep the e-mail address. That tag works > only > when embedded within a HTML document. > [YQ]: do you mean the "editor" in the model? I meant to say you should drop the word “mailto:"; from the list of e-mail addresses of the editors. That tag works inside a HTML document, not inside a YANG model. > > A pyang compilation of the model with —ietf and —lint option was clean. > However, there seems to be an issue with the example in Section 11. > Instead of > actually including an example, it is showing the path to what should have > been > the example. I was therefore not able to validate the example against the > model. > [YQ]: I can't find this example in OpenConfig Github anymore, so removed this > section. Hmm. That leaves the model without any examples, and examples are important. I was able to find the example in an earlier version of your own draft (01). I would suggest that you take that example and modify it to make it compatible with the latest version of the model. If you need help, let me know. > > Checking references for intended status: Proposed Standard > > ---------------------------------------------------------------------------- > > (See RFCs 3967 and 4897 for information about using normative > references > to lower-maturity documents in RFCs) > > == Unused Reference: 'I-D.ietf-netmod-sub-intf-vlan-model' is defined on > line 1620, but no explicit reference was found in the text The reason you are getting this error is because you are lacking a normative reference to I-D.ietf-netmod-sub-intf-vlan-model in the document. Goes back to the nit I point up earlier. I would suggest that once you have fixed all the issues in the draft, that you re-run idnits locally on the machine. > > -- No information found for draft-ietf-netmod-intf-ext-yang - is the name > correct? > > -- Possible downref: Normative reference to a draft: ref. > 'I-D.ietf-netmod-intf-ext-yang' > > -- No information found for draft-ietf-netmod-sub-intf-vlan-model - is > the > name correct? > > -- Possible downref: Normative reference to a draft: ref. > 'I-D.ietf-netmod-sub-intf-vlan-model' > > [YQ]: there is dependency on both 'I-D.ietf-netmod-intf-ext-yang' and > 'I-D.ietf-netmod-sub-intf-vlan-model'. > > Summary: 0 errors (**), 0 flaws (~~), 9 warnings (==), 5 comments > (--).
_______________________________________________ rtgwg mailing list [email protected] https://www.ietf.org/mailman/listinfo/rtgwg
