Re: [yang-doctors] Yangdoctors early review of draft-ietf-i2rs-rib-data-model-09

> On Feb 20, 2018, at 7:24 AM, Amit Dass <amit.dass@ericsson.com> wrote:
> 
> Hi Ebben,
> 
> I have updated the draft based on your comments. Could you please have a look at the same and provide your feedback?

The indentations are all over the place for the new references that have been added. 

More importantly, and this discussion is still open in front of YANG doctors, adding a reference statement to an import statement, seems to imply a import by revision. As an example, the import of ietf-interfaces has a reference to RFC 7223. But we know that ietf-interfaces is going to updated soon by whatever RFC number gets assigned to rfc7223bis. Expect further guidance on this.

Also, I do not see normative references to RFC 6991, and RFC 7223 in the text of the document. If this is not clear, see rfc7223bis, where in Section 3.1, there is normative reference to RFC7224 for the iana-if-types module that is imported by the ietf-interfaces module.

Cheers.

> 
> https://datatracker.ietf.org/doc/draft-ietf-i2rs-rib-data-model/
> 
> Best regards,
> Amit
> 
> -----Original Message-----
> From: Ebben Aries [mailto:exa@juniper.net] 
> Sent: Thursday, January 18, 2018 9:33 AM
> To: yang-doctors@ietf.org
> Cc: i2rs@ietf.org; draft-ietf-i2rs-rib-data-model.all@ietf.org; ietf@ietf.org
> Subject: Yangdoctors early review of draft-ietf-i2rs-rib-data-model-09
> 
> Reviewer: Ebben Aries
> Review result: On the Right Track
> 
> 1 module in this draft:
> - ietf-i2rs-rib@2017-12-05.yang
> 
> No YANG validation errors or warnings (from pyang 1.7.3 and yanglint 0.14.59)
> 
> 0 examples are provided in this draft (section 3.12 of
> draft-ietf-netmod-rfc6087bis-15)
> 
> Module ietf-i2rs-rib@2017-12-05.yang:
> - yang-version statement missing - should be 1.1
> - prefix 'iir' is recommended for this module, would 'rib' suffice better?
> - import "ietf-inet-types" should reference RFC 6991 per (not as a comment)
>  https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#section-4.7
> - import "ietf-interfaces" should reference RFC 7223 per
>  https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#section-4.7

This should reference rfc7223bis.

> - import "ietf-yang-types" should reference RFC 6991 per
>  https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#section-4.7
> - Since this module imports "ietf-interfaces", a normative references must be
>  added per
>  https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#section-3.9
> - prefix "if" in the import "ietf-interfaces" can remove quotes to remain
>  consistent with other imports
> - Remove WG Chairs from contact information per
>  https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#appendix-C
> - Module description must contain most recent copyright notice per
>  https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#appendix-C
> - Module description should contain note to RFC Ed. and placeholder reference
>  to RFC when assigned
>  https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#appendix-C
> - Add placeholder reference and note to RFC Ed. for RFC when assigned
>  https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#appendix-C
> - Security Considerations should be updated to reflect new template at
>  https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines
> - Section 1.2 should be replaced with reference to
>  draft-ietf-netmod-yang-tree-diagrams-02 rather (as-is in other i2rs YANG
>  drafts in progress) per
>  https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-15#section-2.5.1
> - This module contains '12' features.  While it is understood the purpose of
>  these features in the module, take precaution as to complexity for clients
>  if they need to understand >= quantity of features per module in use on a
>  network-element.
> - A few comments exist that are either unecessary or redundant.  Encode the
>  comment intent rather in description fields if need be.
> - Per NMDA, which datastores are targeted for the module?  Will all RPC
>  operations be acting upon the dynamic/ephemeral datastore?  It is not clear
>  to me if the intention is to be persistent or ephemeral
> 
> General comments/Nits:
> - references to 'def' could be expanded out to 'definition'
> - references to 'decap' could be expanded out to 'decapsulation' for
>  readability (across definitions and descriptions)
> - Follow consistent capitalization of 'RIB' throughout document text.  Mixed
>  use of 'Rib' and 'rib' exists (Outside of YANG node lowercase definitions).
> - Is it necessary to prefix all nodes under the nexthop container with
>  "nexthop-"?
> - Section 2.5 - route-add RPC - text mentions it is required that the nh-add
>  RPC be called as a pre-requisite however if the nh already exists and the
>  nexthop-id is known, this should not be necessary.  In addition, the text
>  reads 'or return' which should rather be a result of querying the
>  appropriate node in the data tree.
> - In 'IANA Considerations' - s/This document requests to register/This
>  document registers/
> 
> _______________________________________________
> yang-doctors mailing list
> yang-doctors@ietf.org
> https://www.ietf.org/mailman/listinfo/yang-doctors

Mahesh Jethanandani
mjethanandani@gmail.com