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

From: Juergen Schoenwaelder <[email-masked]>
Date: Tue, 13 Apr 2010 13:55:31 +0200

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/>

---

From: Ben Campbell <[email-masked]>
Date: Tue, 13 Apr 2010 08:58:22 -0500

Thanks, this addresses all of my concerns. Additional comments below. 

>> 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.

[...]

> our goal to be more strict than the underlying specifications.

Okay.

[...]

> I am open to be convinced to remove the ABNF if someone feels strongly

I don't feel strongly--it was just a mild general concern that duplicate _normative_ text can lead to future errors if, as you say, the RFC gets updated. But if you see value in having the ABNF in the description, that's okay with me. At the most, it might be worth putting a comment in the description to see the RFC for the full normative definition.

[...]

Thanks!

Ben.

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Tue, 13 Apr 2010 19:11:37 +0200

> I don't feel strongly--it was just a mild general concern that duplicate _normative_ text can lead to future errors if, as you say, the RFC gets updated. But if you see value in having the ABNF in…

Since there is an explicit reference to RFC 3336, I think this is
covered. Many of the data types document formats that are described in
other documents and there is a general trade off what to include and
what to leave out and to include by reference. I guess it is at the
end a judgement call and at this stage of the process, I simply prefer
to minimize changes.

/js