Last Call Review of draft-ietf-netconf-nmda-netconf-03
review-ietf-netconf-nmda-netconf-03-yangdoctors-lc-aries-2018-02-22-00

1 module in this draft:
- ietf-netconf-nmda@2018-02-05.yang

YANG validation errors or warnings (from pyang 1.7.3 and yanglint 0.14.69)
- ietf-netconf-nmda@2018-02-05.yang:171: warning: RFC 6087: 4.10,4.12:
  statement "enum" should have a "description" substatement (From pyang 1.7.3)

0 examples are provided in this draft (section 3.12 of
draft-ietf-netmod-rfc6087bis-18)

Module ietf-netconf-nmda@2018-02-05.yang:
- Note: For the following imports, there are no references to the supporting
  documents as suggested in rfc6087bis however this item is currently under
  discussion on both usefulness/possible formatting
  - import "ietf-yang-types" should reference RFC 6991 per (not as a comment)
    https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-18#section-4.7
  - import "ietf-inet-types" should reference RFC 6991 per (not as a comment)
    https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-18#section-4.7
  - import "ietf-datastores" should reference I-D.ietf-netmod-revised-datastores per
    https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-18#section-4.7
  - import "ietf-origin" should reference I-D.ietf-netmod-revised-datastores per
    https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-18#section-4.7
  - import "ietf-netconf" should reference RFC 6241 per
    https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-18#section-4.7
  - import "ietf-netconf-with-defaults" should reference RFC 6243 per
    https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-18#section-4.7
- Module description, revision and feature definition should contain note to
  RFC Ed. to change placeholder reference to RFC when assigned
  https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-18#appendix-C
- Security Considerations looks good and is adjusted to account for NETCONF
  only as well as addressing the additional RPCs introduced
- L82: suggested replacement of text:
  s/, i.e., the filter criteria are logically ANDed/. Multiple filters are
  processed as a logical AND operation/
- L127: suggested replacement of text:
  s/then the get-data/the get-data/


General comments/nits on draft text:
- As mentioned above, there are 0 examples in this draft.  There should be XML
  RPC examples of the 2 newly defined RPCs as well as usage examples of the
  augments of RFC6241 RPCs
- :with-origin    urn:ietf:params:netconf:capability:with-origin:1.0
  Wouldn't it make more sense to have this URN defined in
  I-D.ietf-netmod-revised-datastores rather?  In addition, should this
  capability and feature be rather defined as 'origin' in the 'ietf-origin'
  module?  As defined today, this feature is used as a clause for
  'origin-filter' as well as 'with-origin' inputs to get-data however these
  inputs appear to be mutually exclusive.
  IMHO, this should be detached from this specific draft which focuses on
  NETCONF specific base extensions for datastores as the returning and
  filtering of origin metadata can be transport independent.
- L308: suggested replacement of text:
  s/parameter is optional to support/parameter is optional and dependant off
  of the servers support for the 'origin' feature/ (dependant off the previous
  comment)
- L320: "ietf-netconf-nmda" (see Section 4). - This should be an actual
  reference as in line #306
- L345: "default-operation" parameter of the <edit-config> operation - This
  should at a bare minimum call reference to the appropriate section in
  RFC6241
- To the previous point, since there is a large amount of reference and
  comparison to RFC6241 definitions, any references should be tagged
  appropriately