Hi Ebben, Can you please advise whether you are OK with revision 11 which addresses your review comments? https://tools.ietf.org/html/draft-ietf-mpls-base-yang-11 Regards, Tarek On 9/12/19, 9:18 AM, "Tarek Saad" <tsaad.net@xxxxxxxxx> wrote: Hi Ebben, Thanks for your review and comments. We have uploaded a new revision of the document that addresses the comments. Inline for more details. On 8/18/19, 4:19 PM, "Ebben Aries via Datatracker" <noreply@xxxxxxxx> wrote: Reviewer: Ebben Aries Review result: On the Right Track 1 module in this draft: - ietf-mpls@xxxxxxxxxxxxxxx No YANG compiler errors or warnings (pyang 2.0.1, yanglint 1.1.40, confdc 6.6.3) Module ietf-mpls@xxxxxxxxxxxxxxx: -------------------------------------------------- - Remove WG Chairs from contact information per https://tools.ietf.org/html/rfc8407#appendix-B [TS]: done. - 'ietf-interfaces' import should reference RFC8343 rather [TS]: updated. - Must clause for start/end-label is incorrect. Would suggest moving this must statement underneath the `leaf end-label` as well: e.g. leaf end-label { type rt-types:mpls-label; must '. >= ../start-label' { error-message "The end-label must be greater than or equal " + "to start-label"; } description "Label-block end"; } [TS]: OK, I moved this check to under end-label. I added similar check for start-label too. - Use of 'state' container under '/routing/mpls/label-blocks/label-block/state' These nodes could sit as r/o nodes by the looks of it directly under the label-block list. In addition, do these nodes need '-count' suffixes? Should they rather be of type `yang:counter32`? [TS]: removed the 'state' container and updated to directly go under list. Also changed to type to yang:counter32. See: https://github.com/netmod-wg/FAQ/wiki/NMDA-Modelling-FAQ - Is there any intention to define any surrounding features? [TS]: updated. General comments/minor nits on the draft/modules: -------------------------------------------------- - Section 1: s/feauture/feature/ - Section 2.1: s/the the/the/ - Section 2.1: 'labeled' vs. 'labelled' - Section 2.4: s/followinig/following/ - Module line 354/367: This is for the 'active-route' action statement rather [TS]: addressed the typos, thanks. Regards, Tarek (for authors)
