Last Call Review of draft-ietf-netconf-rfc7895bis-04

Request Review of draft-ietf-netconf-rfc7895bis-04
Requested rev. 04 (document currently at 07)
Type Last Call Review
Team YANG Doctors (yangdoctors)
Deadline 2018-02-22
Requested 2018-02-01
Requested by Mahesh Jethanandani
Draft last updated 2018-02-19
Completed reviews Yangdoctors Last Call review of -04 by Reshad Rahman (diff)
Genart Last Call review of -06 by Erik Kline (diff)
Opsdir Last Call review of -06 by Linda Dunbar (diff)
Assignment Reviewer Reshad Rahman
State Completed
Review review-ietf-netconf-rfc7895bis-04-yangdoctors-lc-rahman-2018-02-19
Reviewed rev. 04 (document currently at 07)
Review result Ready with Nits
Review completed: 2018-02-19


YANG Doctor review of draft-ietf-netconf-rfc7895bis-04 by Reshad Rahman.

1 module in this draft:
- ietf-yang-library@2018-01-26.yang

YANG validation on 2018-02-18 reports 2 errors but no warnings. This comes from yanglint 0.14.69 and seems to be a tool issue:
err : Module "ietf-yang-library" in another revision already implemented.
err : Module "ietf-yang-library" parsing failed.

2 examples are provided in this draft (Appendices B and C).

Module ietf-yang-library@2018-01-26.yang:
- typedef revision-identifier. The definition allows invalid dates such as 2018-99-99, has there been any consideration to use a more restrictive definition, e.g. ‘\d{4}-[0-1][0-9]-[0-3][0-9]’ (I realize this still allows incorrect dates)?  Also the revision timestamp is just a date which follows YYYY-MM-DD format, isn’t there an existing common typedef (as there is for date-and-time) which can be reused?
- grouping implementation-parameters should be renamed module-implementation-parameters?
- The groupings defined are used only once. For yang-library-parameters I see why this is done since it can be reused. But for others such as implementation-parameters, I don’t see why a grouping is needed. Maybe this is just authors’ preference to regroup information.
- Description of leaf module in list deviation: by “self-referential” I assume that this means that the reference can not refer to the ietf-yang-library module? While this may seem obvious I believe it’d be good to spell it out.
- list module-set. Since this is for schemas, consider renaming it to something along the lines of schema-module-set?
- Use of “non-import module” in a few descriptions. Should this be “non import-only modules”?

- Section 1, 2nd paragraph s/informaton/information/
- Section 1, 3rd paragraph, add “” quote around /module-states.
- Section 1, 5th paragraph s/Since the YANG library contents changes/Since the YANG library contents change/?
- Section 4, 2nd paragraph “…that individual datastore schema share..”. Should that be “… that individual datastore schemas share..”?
- Appendix B and C. The examples use YANG models which are still being updated (hence the note to RFC Editor). Why not use a YANG model which is stable, is this because of need to have an NMDA compliant module in the examples?