ietf
[Top] [All Lists]

Re: Gen-ART LC review of draft-ietf-netmod-yang-types-07

2010-04-13 10:07:17
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(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/ietf

<Prev in Thread] Current Thread [Next in Thread>