Subject: [netmod] review of draft-ietf-netmod-rfc6020bis-08.txt

From: Andy Bierman <[email-masked]>
Date: Tue, 27 Oct 2015 10:51:07 -0700

Hi,

Here are my review comments on YANG 1.1.
IMO the document is almost ready for publication.
I plan to implement YANG 1.1 in YumaPro tools soon.

Andy

---

From: Nadeau Thomas <[email-masked]>
Date: Wed, 28 Oct 2015 07:55:42 -0400

> On Oct 27, 2015:1:51 PM, at 1:51 PM, Andy Bierman <[email-masked]> wrote:
> 
> Hi,
> 

	What remains before it is ready?

	—Tom

> <review1.txt>_______________________________________________
> netmod mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netmod

---

From: Andy Bierman <[email-masked]>
Date: Wed, 28 Oct 2015 09:22:14 -0700

>         What remains before it is ready?
I think my review comments specify that.

Andy

---

From: Nadeau Thomas <[email-masked]>
Date: Wed, 28 Oct 2015 12:47:04 -0400

I am looking to get a feel for the changes will be ready.   
Looking at the comments, these seem to be more or less editorial changes.  Can you 
please give a sense of when they might be ready?

	—Tom

Sent from my Apple ][

> On Oct 28, 2015:12:22 PM, at 12:22 PM, Andy Bierman <[email-masked]> wrote:
> 
> 
> 
> On Wed, Oct 28, 2015 at 4:55 AM, Nadeau Thomas <[email-masked] <mailto:[email-masked]>> wrote:
> 
> 
> > On Oct 27, 2015:1:51 PM, at 1:51 PM, Andy Bierman <[email-masked] <mailto:[email-masked]>> wrote:
> >
> > Hi,
> >
> > [email-masked] <mailto:[email-masked]>
> > https://www.ietf.org/mailman/listinfo/netmod <https://www.ietf.org/mailman/listinfo/netmod>
> 
>

---

From: Andy Bierman <[email-masked]>
Date: Wed, 28 Oct 2015 09:58:58 -0700

Hi,

I don't know when YANG 1.1 will be ready.

Andy

---

From: Martin Bjorklund <[email-masked]>
Date: Fri, 30 Oct 2015 16:45:50 +0100 (CET)

Hi Andy,

Thank you for this review!  Comments inline.

Andy Bierman <[email-masked]> wrote:
> Review of draft-ietf-netmod-rfc6020bis-08.txt
> Andy Bierman
> 2015-10-27
> 
> Sec 1:
>   - text about 'proposed to be used' should be 'going to be used';
>     RESTCONF and JSON drafts part of same WG charter as YANG 1.1

The text actually says:

   Since the publication of YANG version 1
   [RFC6020], YANG has been used or proposed to be used for other
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   protocols (e.g., RESTCONF [I-D.ietf-netconf-restconf] and CoMI
              ^^^^

   [I-D.vanderstok-core-comi]).  Further, other encodings than XML have
   been proposed (e.g., JSON [I-D.ietf-netmod-yang-json]).

So I think this is ok.

>  - RESTCONF needs to be in scope
>      "Other protocols and encodings are possible, but out of scope for this
>       specification."

Are you proposing that we should define rules for RESTCONF in this
document?  I don't think that would be a good idea; the consensus is
rather to remove protocol-specific text from this document.

> Sec. 1.1
>    o  Made "when" and "if-feature" illegal on list keys, unless the
>       parent is also conditional, and the condition matches the parent's
>       condition.
> 
>   - This contradicts text in
>    7.20.2:
>    A leaf that is a list key MUST NOT have any "if-feature" statements.
>    7.21.5:
>    A leaf that is a list key MUST NOT have a "when" statement.
> 
>   - Which is correct?

You're right, it seems this issue was never really resolved.

See the thread:
https://www.ietf.org/mail-archive/web/netmod/current/msg11537.html

the last email on this issue is:
https://www.ietf.org/mail-archive/web/netmod/current/msg11570.html

>   - The 7.* MUST NOT text is not backward compatible with YANG 1.0
>     I think the WG agreed on what the bullet in 1.1 says, not the
>     new restrictions in 7.*

In this email thread it was pointed out that this is very
difficult to check for "when", and at least non-trivial for
"if-feature".

> Sec. 4.2.1, para 2:
> 
>    A server may implement a number of modules, allowing multiple views
>    of the same data, or multiple views of disjoint subsections of the
>    server's data.  Alternatively, the server may implement only one
>    module that defines all available data.
> 
>  - What does 'multiple views of the same data' really mean?

I think the idea is that it fine to implement a native module for
e.g. interface config, in addition to the standard model.

>  - Should 'may' be 'MAY'?

Actually, since the enitire section 4 is not marked as:

   This non-normative section is intended to give a high-level overview
   of YANG to first-time readers.

I think we should remove all 2119-keywords from section 4.

> Sec. 5.6.4: para 2:
> 
>    A server MUST NOT implement more than one revision of a module.
> 
>  - IMO this should a SHOULD NOT or 'MUST NOT advertise' instead
>    of MUST NOT implement

There is an ongoing thread on this topic, the last msg is:

https://www.ietf.org/mail-archive/web/netmod/current/msg14211.html

The question in this msg is not yet answered.

>    Example of possible exception:
>     - rev N-1 has object needed by application
>     - rev N has that object set to status 'obsolete' (and removed)
>     - rev N also has other objects unrelated to the obsolete object
>       but desired for the product feature set

In this case revision N should be implemented.

> Sec. 5.6.4:
>   - para 4: the rules for populating the YANG library should probably be
>     in the yang-library draft
>   - the example on page 35 - 37 should probably be in the
>     yang-library draft

Maybe, but I think this document might seem under-specified unless we
define these rules here.

> Sec. 5.6.5.  Announcing Conformance Information in NETCONF
> 
>   - This should really be in a NETCONF 1.2 spec

Maybe, but we need to work with NETCONF 1.1 now.

>   - RESTCONF draft defines ietf-yang-library requirements for RESTCONF.
>     Why should YANG 1.1 define NETCONF ietf-yang-library requirements?

B/c this document defines both the language and the NETCONF mapping.

> Sec. 5.7: Datastore Modification
> 
>   - This section is NETCONF specific for no apparent reason

Ok.  I suggest s/NETCONF protocol messages/network management protocol
messages/

> Sec. 6.4.1: bullet 5
> 
>    o  The set of variable bindings is empty.
> 
>   - This is not true.  The NACM data model defines the USER variable.
>     This text should say that YANG modules MAY define variable bindings
>     associated with conformance for that module (like NACM does).

No, the USER variable is only available in expressions of type
nacm:node-instance-identifier.   It cannot be used in must/when
expressions.

> Sec. 6.4.1: XPath accessible tree
> 
>   - I object to expanding the context of XPath evaluation for
>     notification and rpc/input, rpc/output.  This is too complicated
>     to implement and under-specified, resulting in poor interoperability.

This is issue Y04, and it was discussed at the WG interim 2014-06-11.

>   - What is the justification for this expansion of complexity?
>     An tool cannot reproduce the same result anymore for
>     <rpc>, <rpc-reply> or <notification> offline validation.

It couldn't do that earlier either, if there were leafrefs in the
data.

I agree with all your concerns below.  I would prefer Y04-01, but that
would brake RFC 6643 :(

Maybe we should consider doing this only for the 'path' statement.  It
has the same set of issues of course.

>  - Notification context expanded from notification message to message +
>    running config + operational state;  When is the server supposed
>    to gather and check all the data nodes? Event time? Buffer time?
>    Send time? What if operational state or config changes while the
>    notification is in the replay buffer?
> 
>  - RPC input context expanded from input message to message +
>    running config + operational state;  When is the server supposed
>    to gather and check all the data nodes? Parse time? Buffer time?
>    Invocation time?
> 
>  - RPC output context expanded from input message to message +
>    running config + operational state;  Is the server supposed to
>    wait for operational state that may change as a result of the
>    action or rpc being invoked?
> 
>  - The expanded contexts for notification and rpc statements
>    require coupling of the notification and RPC processing
>    implementation components to the datastore and operational data.
>    This is a big change from current NETCONF requirements.
>    YANG should not be making any changes to NETCONF, but it does
>    in several places, effectively creating a new undeclared
>    version of the NETCONF protocol
> 
>  - It is not clear if "action" and "notification" statements
>    nested in the data tree are visible at all for XPath evaluation.
>    (They MUST NOT be visible)

Agree.  I think this is covered since the accessible tree is made up
of data nodes, and action/notification are not data nodes.

>  - NETCONF <get> and <get-config> "filter" element needs clarification
>    wrt/ action and notification within the data tree

Why?  These nodes are not part of the data tree (datastore).

>  - NETCONF and RESTCONF notifications "filter" element needs clarification
>    wrt/ action and notification within the data tree
>
> Sec. 7.5.3: must: last para
> 
>    Also note that the XPath expression is conceptually evaluated.  This
>    means that an implementation does not have to use an XPath evaluator
>    in the server.  How the evaluation is done in practice is an
>    implementation decision.

Is there a problem with this text?

> Sec. 7.5.4.2.  The error-app-tag Statement
> 
>    The "error-app-tag" statement, which is optional, takes a string as
>    an argument.  If the constraint evaluates to false, the string is
>    passed as <error-app-tag> in the <rpc-error> in NETCONF.
> 
>  - all the error-message and error-app-tag sub-sections apply to
>    RESTCONF as well as NETCONF.  They share the same error reporting
>    structure

Agree.  But this document does not define RESTCONF.

> Sec. 7.5.8:
> Sec. 7.6.7:
> Sec. 7.7.9:
> Sec. 7.9.6:
> Sec. 7.10.3:  NETCONF <edit-config> Operations
> 
>  - Can these sections be changed to 'Configuration Datastore Edit
>    Operations' and be generic so they focus on the contents of
>    the datastore before and after the edit, without specifying
>    <edit-config> as the edit operation

Hmm, I think these sections are really edit-config-specific.  YANG
Patch defines its own syntax, for example.

>  - RESTCONF needs to be supported, or all this should be moved
>    to NETCONF 1.2

RESTCONF *is* supported, but it is not *specified* in this document.

> Sec. 7.6.1.  The leaf's default value
> 
>  - this section should mention that a false if-feature
>    or false when-stmt removes the schema node from the
>    tree, which overrides all this text. There is no default
>    value if there is no schema node

Howabout:

  Note that if the leaf or any of its ancestors has a "when" condition
  or "if-feature" expression that evaluates to "false", then the
  default value is not in use.

(and ditto for leaf-list default)

>  - It is confusing that 'must' and 'when' work very
>    differently wrt/ default. must-stmt does not ignore defaults
>    but 'when' overrides and removes the 'when', so it needs to
>    be evaluated before the 'must' stmt:
> 
>    leaf confusing {
>      must "/some/val=4";
>      when "/some/other/value/test";
>      default 42;
>      type int32;
>    }

Yes.

> Sec. 7.7.2.  The leaf-list's default values
> 
>  - this section represents 2 new requirements to compilers
>    that should be more apparent:
> 
>    1) remembering the order of "default" statements
>    2) the default only applies if 'min-elements 0' is in effect

Can you suggest some text to clarify this?

>  - what is the rationale for not applying the default if
>    min-elements is > 0?

B/c the default applies only if no instance exists, and if
min-elements is > 0, there MUST exist an instance.

>  It looks like the leaf/container rule
>    that mandatory and default are not allowed together is
>    being applied here.  This should be more clear here
> 
> Sec. 7.11.  The anyxml Statement
> 
>  - The 'anyxml' statement should be deprecated.
>    If not, this section needs to provide rationale for
>    its 'current' status, given that 'anydata' has been added
>    No use cases are provided for anyxml in the draft.

It should be used when you need to model some XML data.  It is used in
RFC 6241.

>  - There are few if any servers that support anyxml the way
>    it is defined here, and that is not likely to change.

Servers support the data models they advertise.  anyxml is almost
never used in any data model, except for ietf-netconf.  A server that
supports ietf-netconf + YANG will support the subset of XML that is
necessary for YANG.

>    The draft does not specify how the XML must be parsed,
>    stored, or rendered later, if it is part of configuration

Do you want to add more rules around this?

> Sec. 7.15.  The action Statement
> 
>  - an action must appear directly within a container or a
>    list, an 'augment' or a 'uses' statement.
>    Why is 'case' statement left out of direct usage and
>    only allowed via 'uses' or 'augment'?

It is explicitly not allowed in uses:

  Since an action cannot be defined an the top-level of a module or in
  a case statement, it is an error if a grouping that contains an
  action at the top of its node hierarchy is used at the top-level of
  a module or in a case definition.

As for augments, the text actually is broken; it doesn't mention
action or notification at all.  I suggest we add:

  If the target node is a container or list node, the "action" and
  "notification" statements can be used within the "augment"
  statement.

to section 7.17.

(and the reason for not allowing it is that it is not clear what it
means.  for example:

    container foo {
      choice c {
        case a {
          action doit { ... }
        }
        case b {
          notification it-happened { ... }
        }
      }
    }
)

>  - para 2: groupings are not reusable if they contain actions,
>    since they are not allowed in rpc, notification, or action
>    This should be pointed out in the section on groupings

How about this NEW text in 7.12 The grouping Statement:

  Note that if a grouping defines an "action" or a "notification" node
  in its hierarchy, then it cannot be used in all contexts.  For
  example, it cannot be used in an rpc definition.  See ^action^ and
  ^notification^.

>  - the reason for the restriction on top-level action is not
>    given.  The syntax would support this, and the identifier
>    namespace is shared (same as rpc).

They are different:

   The "rpc" statement is used to define an RPC operation.

   The "action" statement is used to define an operation connected to a
   specific container or list data node.

>  - the context nodes for action input and output are very different
>    this could be more clear

Can you suggest some text to clarify this?

>  - is the server required to process XPath or constraints on the
>    output at some point in the execution of the action?

This is not different from rpc output (or norifications).  I think it
is the server's responsibility to send output / notifs that are
syntactically and semanticically valid.  *how* this is done is an
implementation detail.  (FWIW our code doesn't check such constraints;
we leave it to the implementor to do the right thing).

>  - there is no rationale given why the ping operation should
>    be done as an action instead of an RPC

It can be done in both ways.  I think such guidelines are out of scope
for this document.

>    list interface {
>        key "name";
> 
>        leaf name {
>          type string;
>        }
> 
>        action ping {
>          input {
>            leaf destination {
>              type inet:ip-address;
>            }
>          }
>          output {
>            leaf packet-loss {
>              type uint8;
>            }
>          }
>        }
>      }
> 
>  Why not use an RPC?
>  No rationale for both ways to implement an operation
>  like 'ping' is given.
> 
>    rpc ping {
>      input {
>        leaf itf-name {
>          mandatory true;
>          type if:interface-ref;
>        }
>        leaf destination {
>          type inet:ip-address;
>        }
>      }
>      output {
>        leaf packet-loss {
>          type uint8;
>        }
>      }
>    }
> 
> Sec. 7.16.  The notification Statement
> 
>    If a leaf in the notification tree has a "mandatory" statement with
>    the value "true", the leaf MUST be present in a notification
>    instance.
> 
>  - Why is this apply to leaf but not container or choice?
>    They should be mentioned here as well.

... and (leaf-)list min-elements.

This sentence (about mandatory leafs) is present also for rpc
output,input and notif.

But maybe the best solution would be to remove this special sentence -
it is already covered by the text in 8.1 - and that text is more
complete.

But I noticed that the text in 8.1 doesn't mention mandatory choices.
I suggest:

OLD:

   o  The "mandatory" constraint is enforced for leafs, unless the node
      or any of its ancestors has a "when" condition or "if-feature"
      expression that evaluates to "false".

NEW:

   o  The "mandatory" constraint is enforced for leafs and choices,
      unless the node or any of its ancestors has a "when" condition
      or "if-feature" expression that evaluates to "false".

>  - No rationale is given for defining notifications within data nodes
>    vs. top-level notification-stmt

See above for actions.

>  - It is not clear how RFC 5277 <filter> can be applied to the
>     notification message that is nested within data, given the
>    location of the 'notification' root within the data tree is
>    not static, and different for each notification.

Why wouldn't a normal filter work?

For example, for the last example in 7.16.3 you can do:

  <filter>
    <interfaces xmlns="http://example.com/schema/interfaces">
      <interface>
        <name>eth1</name>
      </interface>
    </interfaces>
  </filter>

in order to receive notifs for eth1 only.

> Sec. 7.17.  The augment Statement
> 
>  - can augment path-stmts contain nested 'action' or 'notification'
>    nodes and subnodes, like for rpc-stmt?  If, so then the
>    sub-statements are restricted by context, e.g., action-stmt
>    cannot be added to /foo/bar/action/input/baz.

Above I suggested we add:

  If the target node is a container or list node, the "action" and
  "notification" statements can be used within the "augment"
  statement.

I am not sure if that covers your concern?

> Sec. 7.18.2.  The base Statement
> 
>   - There are no examples or use-cases given for multiple base
>     statements. An example should be added.

Ok.

>  - Para 1 says:
>       If multiple "base" statements are present, the identity is derived
>       from all of them.
> 
>    The draft does not say how this is done. It is not clear how
>    a particular identity-stmt has multiple bases and how a server
>    determines an identify is derived from multiple bases. Examples
>    of a 'pass' and 'fail' for this test should be given.

This section simply defines what "derived from" means.  An identity
can be derived from zero, one or many other identites.

> Sec. 7.19.  The extension Statement
> 
>    "YANG statements in extensions MUST follow the syntactical rules
>     in Section 14."
> 
>   - What is the rationale for this rule? Extension statements
>     should be outside the scope of YANG. They should just follow the
>     syntax rules in sec. 6.

The idea is that it is fine to use normal core YANG statements as
substatements to an extension, but then they must follow the rules for
these statements.  If you need some other syntax, then you have to
define a new extension.  For example:

      // legal
      ex:arg-type {
        type enumeration {
          enum "none";
          enum "all";
        }
      }
        
      // illegal
      ex:arg-type {
        type enumeration {
          pattern "none|all";
        }
      }
        
>  - This says YANG syntax MUST be preserved, but YANG semantics
>    can be ignored or changed.  Is that the intent?

Probably not.  I think semantics must be preserved as well.  Do you
agree?

>    "An extension can allow refinement (see Section 7.13.2) and deviations
>    (Section 7.20.3.2), but the mechanism for how this is defined is
>    outside the scope of this specification."
> 
>  - This text about refinement and deviations is not clear.
>    Is it MUST, SHOULD, or MAY? It is normative, but unspecified.

It's like:

  The substatements of an extension are defined by the "extension"
  statement, using some mechanism outside the scope of this
  specification.

How does a parser know whih substatements are allowed in a
specific extension?  There is no standard formal way to define this.
Same with refinement and deviations.

> Sec. 7.20.2.  The if-feature Statement
> 
>  - This section does not define how "(" if-feature-expr ")" is
>    evaluated, or how the precedence is evaluated if parens are used.
>    Evaluated of nested parens is not defined either.

Good catch.  I suggest:

OLD:

   The if-feature boolean expression syntax is formally defined by the
   rule "if-feature-expr" in Section 14.  When this boolean expression
   is evaluated, the operator order of precedence is (highest precedence
   first): "not", "and", "or".

NEW:

   The if-feature boolean expression syntax is formally defined by the
   rule "if-feature-expr" in Section 14.  Parenthesis are used to group
   expressions.  When the expression is evaluated, the order of
   precedence is (highest precedence first): grouping (parenthesis),
   "not", "and", "or".

>  - Are operators of the same type evaluated left to right?

This doesn't matter.

>  - should be some examples to show precedence
>    "not foo or bar and baz"
>    "(not foo) or (bar and baz)"
>    "not (foo or bar) and baz"

Ok.

> Sec. 7.21.2.  The status Statement
> 
>    If a definition is "current", it MUST NOT reference a "deprecated" or
>    "obsolete" definition within the same module.
> 
>    If a definition is "deprecated", it MUST NOT reference an "obsolete"
>    definition within the same module.
> 
>  - It is not clear what it means for a definition to reference
>    another definition.

Do you have any suggestions for more explanatory text?

>  - Why is this restriction limited to the same module and not
>    include imported modules?

An imported defintion might get a new status in a new revision.  If
we has this rule across modules, it would mean that a server wouldn't
be able to implement certain combinations of otherwise legal modules.

>  - Don't the 'deprecated' and 'obsolete' values ripple up through
>    the data tree, all the way to the top-level node?

No.

>    Otherwise
>    a current list can have deprecated or obsolete children.

Sure, don't you think that is ok?  Otherwise a single deprecated node
would essentially deprecate the enitre module.

>    It should be explained how status is contained (or not)
>    for nodes within other statements.

I don't understand what you mean.  Can you suggest some text?

>  - What does it mean to use a current grouping that contains deprecated
>    or obsolete nodes?  Is this allowed?

Sure, this is allowed.

> Sec. 7.21.5.  The when Statement
> 
>  bullet 3:
>  - Altering the tree changes means that each when-stmt processes
>    a different XML document.  IMO it is a bad idea to replace a
>    node that is in the tree with a dummy node.  It is OK to create
>    a temp dummy node as an implementation detail to decide if
>    a when-stmt is true or not.

We've discussed this at length on the ML.  I think the current text
reflects WG consensus.  Without this text, it is not clear how to
interpret the theoretical corner case when the expression references
nodes that are being augmented:

   augment /... {
     when "foo != 42";
     leaf foo {
       type int32;
     }
   }

>  "If the XPath expression references any node that also has associated
>  "when" statements, these "when" expressions MUST be evaluated first.
>  There MUST NOT be any circular dependencies in these "when"
>  expressions."
> 
>  - This requirement is not implementable and needs to be removed.
>    The draft should just say what the result is supposed to be
>    (and it does -- all nodes with false when-stmts are removed)

Would it be ok to write "conceptually evaluated"?

This rule was added as part of fixing Y18.  See the issue list for an
example of why it is needed.

>  "Note that the XPath expression is conceptually evaluated.  This means
>   that an implementation does not have to use an XPath evaluator in the
>   server.  The "when" statement can very well be implemented with
>   specially written code."
> 
>  - The rules for how to process a when-stmt contradict this
>    text at the end of the section that when-stmt is only
>    conceptual.  No implementation details should be mandated.

Agreed.

> Sec. 8.1: Constraints on Data
> 
>  - bullets say constraint MUST be true, but this info should really
>    be part of NETCONF, and should say what a NETCONF server should
>    so when a constraint is false

Do you want to make this text *more* NETCONF-specific?  The idea was
that these conditions hold regardless of which protocol is used.

> Sec. 8.2.1.  Payload Parsing
> 
>  - The NETCONF-specific error handling text should be made more generic

I don't see how this can be made more generic.  The text specifies
which NETCONF error messages should be returned.  The equivalenmt for
RESTCONF is HTTP status codes, for example.

> Sec. 8.2.2.  NETCONF <edit-config> Processing
> 
>   - This text should be more generic if possible.
>   - Why do all the NETCONF implementation requirements apply only
>     to <edit-config> and not also <copy-config>?

<copy-config> doesn't contain any of the "operation" or "insert"
attributes.

> Sec. 9.6.4.1.  The enum's Substatements
> Sec. 9.7.4.1.  The bit's Substatements
> 
>  - example of if-feature should be given

Ok.

>  - It should be explicitly stated that if-feature has no effect
>    on auto-numbering

Ok.

>  - what happens if a feature value is changed at boot-time or run-time,
>    causing valid enumeration or bits values within a datastore to
>    suddenly become invalid?  Changing the value set of a leaf or leaf-list
>    with if-feature is very complicated and under-specified

I think this is an implementation detail.  Some implementations don't
even support enabling/disabling features.

Compare with this situation:

  leaf foo {
    if-feature foo-feature;
    ...
  }

  leaf my-i-i {
    type instance-idnetifier;
  }

and then the datastore contains:

   my-i-i = /foo

Now suppose you disable 'foo-feature' in your server.  What happens to
the datastore?

> Sec. 9.9.1.  (Leafref) Restrictions
> 
>  - can leafref require-instance be changed via typedef?

Yes; in general all restrictions can be done via a typedef.

>    typedef foo-ptr {
>      type leafref {
>        path "/some/node/foo";
>      }
>    }
> 
>    typedef foo-ptr2 {
>      type foo-ptr {
>        require-instance false;
>      }
>    }
> 
> Sec. 9.10.5.  (Identityref) Usage Example
> 
>  - Add an example and use-case showing multiple base-stmts
>    for an identityref

I will try to come up with something simple.

> Sec. 9.12.  The union Built-In Type
> 
>  - What happens to a union type when a feature is enabled or disabled,
>    and there are 1 or more bits or enumeration member types that are
>    affected, such that the member type changes? Is this just an
>    implementation detail?
> 
> 
> Sec. 10.1.1.  current()
> 
>  - an example for current() should be given

Ok, NEW:

With this list:

  list interface {
    key "name";
    ...
    leaf enabled {
      type boolean;
    }
    ...
  }

the following leaf defines a must expression that ensures that the
referred interface is enabled:

  leaf outgoing-interface {
    type leafref {
      path "/interface/name";
    }
    must '/interface[name=current()]/enabled = "true"';
  }

> Sec. 10.3.1.  deref()
> 
>  - why is this XPath function needed?
>    No use-cases are explained.
>    The example given shows that deref(.) saves some extra typing
>    from the previous line.  Not very interesting new feature.

If the node is an instance-identifier, it is not possible to check the
target node w/o deref().

>  - what if the first node in the parameter node-set is another
>    leafref?  Does this function keep de-referencing until it
>    gets to a plain leaf?

No.

> Sec. 10.4.1.  derived-from()
> Sec. 10.4.2.  derived-from-or-self()
> 
>  - example for derived from in wrong section

Fixed.

>  - no example given for derived-from-or-self()

Fixed.

>  - no explanation why 1 function would be used instead of the other
>  - these functions do not actually define what it means
>    to be "derived from".

This is explained in 7.18.2.  I will add a reference to that section.

>  What does it mean in terms of base-stmt matching
>    like an identityref?
>  - What if there are multiple bases?  These functions only support 1
>    base.

Correct.  If you want to check that it is derived from both "foo" and
"bar", you'll have to write:

     derived-from(x, "ex-module", "foo") and
     derived-from(x, "ex-module", "bar")

> Sec. 11.  Updating a Module
> 
>  - seems all instances of 'may' need to be changed to 'MAY'

Hmm, I think "may" is correct.  Could have been "can" as well.  MAY is
used to denote something OPTIONAL.  (FWIW, this is unchanged from
6020).

>  - last para:
>    In statements that have any data definition statements as
>    substatements, those data definition substatements MUST NOT be
>    reordered.
> 
>    Why is this requirement in here?  Seems like changing the
>    order of case-stmts within a choice-stmt does not matter.
>    Changing the order of augment-stmts does not matter.

An augment is not a data-def stmt.

>    Why does changing order of any data-def statement break an old client?

For NETCONF, the order matters for rpc/action input/output.  Maybe
some other protocol use the specified order to simplify things?

If that is not important, I think we can at least relax this rule to
only apply to rpc/action input/output - and gorupings, since they can
be used in input/output.  Hmm.

> Sec 12.  Coexistence with YANG version 1
> 
>  - para 4: :"server MAY implement both..." contradicts this text in 5.6.4:
>    "A server MUST NOT implement more than one revision of a module."

"both modules" was supposed to mean "A and B".  I will write:

  If a YANG version 1 module A imports module B without revision, and
  module B is updated to YANG version 1.1, a server MAY implement both
  these modules (A and B) at the same time. 

>  - this section does not address what happens if protocol-accessible
>    objects have changed between the implemented YANG 1.0 module revision
>    and the implemented YANG 1.1 module revision.  Since NETCONF and
>    RESTCONF do not specify YANG language version or YANG module revision
>    in edit payloads or retrieval filters, how does the server even
>    know if a client is asking for the YANG 1.0 vs. YANG 1.1  implementation
>    of a particular data node?

What do you mean with 1.0 vs. 1.1 implementation of a data node?  The
data node will have a single implementation in the server.

> Sec. 18.  Contributors
> 
> OLD:
>     - Andy Bierman (Brocade)
> NEW:
>     - Andy Bierman (YumaWorks)

Fixed.

> Nits:
>  p8: para 1:  s/propsed/proposed/
>  p10: bullet 3: s/defintions/definitions/
>  p37, para 4: s/enconding/encoding/
>  p43, para 3: s/SHOULD not/SHOULD NOT/

Fixed.

/martin