Subject: [netmod] review of draft-ietf-netmod-usage-04

From: Martin Bjorklund <[email-masked]>
Date: Sun, 09 May 2010 00:40:30 +0200 (CEST)

Hi,

I have revied this document, and I have implemented the things that
can be checked automatically in a plugin to pyang (see the --ietf
parameter).

Here are my comments:

3.1

   Each YANG module or submodule contained within an Internet Draft or
   RFC MUST be identified as a 'Code Component'

What does this mean?

http://www.ietf.org/iesg/statement/copyright.html and
http://trustee.ietf.org/license-info/archive/Code-Components-List-4-23-09.txt 
together says that a YANG module is a code component.  Should the
quoted sentence above be removed?

I agree with the next sentence though:

   The strings '<CODE BEGINS>' and '<CODE ENDS>' SHOULD be used to
   identify each Code Component.

3.1

   The module description statement MUST contain the latest approved
   IETF Trust Copyright statement, which is available on-line, in
   section 4 of the Trust Legal Provisions (TLP) document, at:

Does this mean that a YANG module MUST contain the simplified BSD
license?  Is ietf-yang-types not compliant with this guideline (it
contains a reference to the license).

3.2

   particularly to standards containing
   other module modules

s/module modules/YANG modules/  ?

3.7

Is this section a duplicate of 3.1?  Should 3.7 be removed?

4.3

   In general, it is suggested that sub-statements containing default
   values SHOULD NOT be present. For example:

I think we should explicitly list the stmts: 
'status', 'mandatory','min-elements' , 'max-elements',
'config', 'yin-element'.

4.4.

The text:

   Objects SHOULD NOT directly reference NETCONF capabilities, in order
   to specify optional behavior.  Instead, a 'feature' statement SHOULD
   be defined instead of a NETCONF capability, and the 'if-feature'
   statement SHOULD be used within the optional object definition.

is not clear.  Specifically "instead of a NETCONF capability".  Did
you mean that a feture SHOULD be define which references a NETCONF
capability?

4.4

   If the condition associated with the desired semantics is not
   dependent on any particular instance value within the database, then
   an 'if-feature' statement SHOULD be used instead of a 'when'
   statement.

Ok, but this is a tautology - if it is not dependent on a database
value, it will be impossible to use a when statement.

4.4
 
 You should say that you talk about XPath, e.g.:

   The 'attribute' and 'namespace' axis SHOULD NOT be used *in XPath
   expressions*

4.4

   Implicit 'position' function calls within predicates SHOULD NOT be
   used. (e.g., //chapter[42]).

What is the rationale for this?

4.5

  The status statement SHOULD NOT be present if its value is 'current'.
  It MUST be present if its value is 'deprecated' or 'obsolete'.

The second sentence is redundant.

  The revision-date sub-statement (within the imports statement)

  The revision-date sub-statement (within the include statement)

Remove the parenthesis.

4.6.
  
should we add that a revision stmt MUST have a description?

4.6

The text says:

   Modules are often extracted from their original documents and it is
   useful for developers and operators to know how to find the original
   source document in a consistent manner.

which is a nice motivation for ...? I think the recommendation is to
include a reference substatement to the original document in the
revision statement.

(Ok, this is actually what another paragraph says, further down, maybe
reorder the paragraphs?)

4.7.

Do we still recommend a *temporary* namespace?

The examples aren't temporary, are they?

4.8.

  Top-level database objects MUST NOT be mandatory.

I suggest SHOULD NOT.  The argument seems to be that mandatory
top-level object can never work.  If that was true, they should be
illegal in YANG.  But we've had this debate before...

4.11

Minor:

   The 'anyxml' construct MAY be used with caution within configuration
   data.  This may be useful to represent an HTML banner for example.

Is this really an appropriate use case of anyxml?   I think a string
would be better.

4.12.

   The description statement MUST be present in 'rpc' statements
   defining new operations.

Ok, but redundant (already specified in 4.11)

--

In my code, I also check that the statements are in the canonical
order,  but I don't know if we want to make that a MUST in this
document.

/martin

---

From: Andy Bierman <[email-masked]>
Date: Tue, 18 May 2010 10:17:36 -0700

Martin Bjorklund wrote:
> ....
[any deleted comment was added.]

> should we add that a revision stmt MUST have a description?

no -- I put MAY have.

> ...

No -- we still need a value assigned.
I added text that any values already in
the YANG Module Registry MUST NOT be used.

> illegal in YANG.  But we've had this debate before...

No.
Our guidelines are based on what a client developers
can expect for all NETCONF servers. Since no NETCONF
is allowed to boot with an invalid must-stmt, and no server
is actually required to provide a value for a mandatory node,
a standard YANG data model cannot contain any top-level
mandatory nodes.

Standard YANG modules can only use YANG mechanisms
that MUST be supported by all implementations.

> ...

No -- all YANG tools MUST support arbitrary statement ordering,
within the rules of the ABNF. It is too hard to remember
the canonical sub-statement order.

> /martin

Andy

---

From: Martin Bjorklund <[email-masked]>
Date: Tue, 18 May 2010 23:18:48 +0200 (CEST)

> > should we add that a revision stmt MUST have a description?

Ok.

> the YANG Module Registry MUST NOT be used.

Ok.

> that MUST be supported by all implementations.

Yes, that is obvious.  But YANG doesn't say that mandatory
top-level nodes are optional to support.

> within the rules of the ABNF.

That's not a good argument.  All YANG tools MUST support modules w/o a
description statement in all typedefs, yet we say that typedefs MUST
have a description statement.  This rule is there in order to make
better data models.  Using the canonical statement order makes a
module easier to read.  (subjective, I know).

> It is too hard to remember

Fortunately, we have tools to help us remember all the guidelines we
define.

/martin

---

From: Andy Bierman <[email-masked]>
Date: Tue, 18 May 2010 14:50:11 -0700

> top-level nodes are optional to support.

Yes, just like we say identifier lengths above 64 chars
are optional to support. Therefore, standard YANG modules
MUST NOT use identifier lengths > 64 chars.
Same guideline applies here.

> module easier to read.  (subjective, I know).

IMO, it is way too fussy to require a rigid
statement order. Humans do not remember
or agree on any particular 'best canonical order'.

We do not have rigid sub-clause ordering like SMIv2.
People always hated that. YANG is more flexible than that.
We just picked an arbitrary order in the ABNF.
If I knew IETFers would have to follow that order
then I would have reviewed much more carefully.

However, my yangdump program (--format=yang),
will pretty-print a YANG module in canonical order.
It is not complete yet (e.g., comments and string
fragmentation not preserved), but it will be available
online soon. (already in the yuma-client package).

I don't even want to say SHOULD be in canonical order.
That means everyone has to follow it anyway.

Another option is to get nit-picky about properties
vs. child data-def-stmts. E.g., all 'key', 'unique', 'presence',
'min-elements', 'max-elements',, 'description', 'reference',
etc. statements SHOULD be present before any child
data-def-stmts. I agree that scattering these
properties amongst the child nodes would harm readability.

Andy