On Tue, Apr 06, 2010 at 10:59:36PM +0200, Ben Campbell wrote: > I have been selected as the General Area Review Team (Gen-ART) > reviewer for this draft (for background on Gen-ART, please see > http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html). Thanks for your review. I will followup on your comments below, CCing the WG mailing list so that the WG sees the exchange between us. > Please resolve these comments along with any other Last Call comments > you may receive. > > Document: draft-ietf-netmod-yang-types-07 > Reviewer: Ben Campbell > Review Date: 2010-04-06 > IETF LC End Date: 2010-04-09 > IESG Telechat date: (if known) > > Summary: This draft is almost ready for publication as a proposed standard. There are a few minor issues that might should be considered prior to publication. > > Major issues: None. > > Minor issues: > > -- Section 3, model namespace definition: > > (Same comment for section 4) > > Will the registered namespace really include "DRAFT-06"? Should this be replaced with the RFC number? No, this will be removed. (This was an attempt to ensure that names used in Internet drafts do not conflict with the name used by the final RFC version of a document, but we meanwhile concluded that this does not work well and has a tendency to increase confusion.) > -- description for counter32: "A default statement should not be used for > attributes with a type value of counter32." > > Should that be a normative SHOULD NOT? I think SHOULD NOT is the intention here, so I will make the change. Since the word 'attribute' is not a defined term, I suggest to rewrite this to: A default statement SHOULD NOT be used in combination with the type counter32. And likewise for counter64. > -- object-identifier description, 3rd paragraph: > > Does this imply a normative requirement that one SHOULD NOT use this > to model an SMIv2 OI? (and SHOULD instead use > object-identifier-128)? I can add a clarifying sentence so that the paragraph becomes: This type is a superset of the SMIv2 OBJECT IDENTIFIER type since it is not restricted to 128 sub-identifiers. Hence, this type SHOULD NOT be used to represent the SMIv2 OBJECT IDENTIFIER type, the object-identifier-128 type SHOULD be used instead. > -- Section 4, domain-name, description, paragraph 2: "...systems that want to store host names in > schema nodes using the domain-name type are recommended to > adhere to this stricter standard to ensure interoperability." > > should "recommended" be normative? I think I would leave it lowercase unless someone can provide a reference where a normative version of the recommendation to follow RFC 952 rules is written down. Our goal in general is to represent what the underlying technology (DNS in this case) specifies, it is not our goal to be more strict than the underlying specifications. > Nits/editorial comments: > > -- Section 2, 1st paragraph: > > Can you provide a reference for SMIv2 (I assume RFC 2578)? Also, > please expand it on first mention. Added a reference to RFC2578 and RFC2579 and expanded the acronym. > -- zero-based-counter32 description, 2nd paragraph: > > Plurality mismatch between "nodes" and "it". Suggest s/"Schema > nodes"/"A Schema node" fixed > -- date-and-time, pattern and description: > > Which is the normative description for date-and-time? The ABNF in > the description, or the pattern attribute? I assume the second, but > fear the presence of ABNF will make others assume the first. Ideally, they should be consistent - and I hope they are. The ABNF is more detailed - if you read the comments - and copied from RFC 3339. If we make a change, we should completely remove the ABNF from the description and simply leave the pointer to RFC 3339, e.g. For a more detailed description, see section 5.6 of RFC 3336. Since the ABNF is copied, this does not really change much unless RFC 3336 gets updated perhaps. For now, I have left things as they are but I am open to be convinced to remove the ABNF if someone feels strongly about this. > (Comment repeats for zero-based-counter64) fixed as well > -- zero-based-counter32 description, 3rd paragraph: > > ben: s/"wrap it"/"wrap, it" > > (Comment repeats for zero-based-counter64) fixed both > -- section 5: > > The namespaces do not match the text (see comments on the module > namespace strings in sections 3 and 4) fixed, see explanation above > -- section 9.2: > > idnits complains about unreferenced entries in this section. I'm not > sure what to do about it, or if it matters at all, since they are > referenced from the model itself. Yes, these are false alarms. Thanks again for reviewing this document. /js -- Juergen Schoenwaelder Jacobs University Bremen gGmbH Phone: +49 421 200 3587 Campus Ring 1, 28759 Bremen, Germany Fax: +49 421 200 3103 <http://www.jacobs-university.de/> _______________________________________________ Ietf mailing list Ietf@xxxxxxxx https://www.ietf.org/mailman/listinfo/ietf