Re: [netmod] Shepherd review on draft-ietf-netmod-yang-instance-file-format-10

[Kent Watsen <[e-mail]>]

[UPDATE: I just realized that I prematurely stopped the review after the YANG module.  Picking up where I left off…]


Structural Issues:

 - S5 contains an mix of important and unimportant information.   I think that the most important thing to state that the module defines an offline format that MAY contain security sensitive information, and thus safe handling is advised.  Maybe also say something about because the YANG module only defines a “structure”,  the Security Considerations doesn’t follow the template specified in https://tools.ietf.org/html/rfc8407#section-3.7.1).  For instance: s/is designed as a wrapper specifying a format and a metadata header for YANG instance data defined by the content-schema/specifies an offline format/

  - S6.1. the full registration is not inside the <figure>.  (How could happen?)

  - S8.1: I-D.ietf-netmod-yang-data-ext should be listed last, as the number it will be the greater than any other...

  - S8.1: RFC8340 is Informative

  - S8.1: agreed that RFC8525 is Normative, but the only place it it referenced is in a non-normative section…please add a ref to it from a normative section.

  - S8.2: same comment as before re: floating the I-D refs to the end…

  - Appendix C: remove the unnecessary “C.1” section.


Editorial Issues:

  - Appendix B:
     - s/For instance data/Instance data/
     - “...to avoid are listed.” - listed where?  Section reference?
     - In P2, how is the 2nd sentence connected to the 1st?
     - s/may lead to problems/may lead to the following problems:/?

  - Appendix C:
      - Don’t put “Non-Normative” into the Section title (move to 1st sentence of section) 
      - s/We present/This section presents/
      - s/use cases were YANG/use cases where YANG/
      - Actually, this whole sentence is meaningless

I gave up reviewing Section C at C.1.1, since it’s non-normative.  Honestly, I'd remove the entire section but, if you want to keep it, I suggest reviewing it for issues…


Kent // shepherd



> On Mar 30, 2020, at 3:22 PM, Kent Watsen <[e-mail]> wrote:
> 
> 
> As part of the Shepherd writeup, I read the entire draft and found the following issues, which I’d like to see resolved before progressing the document.  Most of these issues should have been caught be the WG and/or Editors...
> 
> 
> Logical Issues:
> 
>  - S3, P8 defines MUSTs inside a SHOULD, a logical contradiction.
>  - In S3, P8 (the P beginning w/ "The name of”), text fails to indicate what SHOULD be done if both “revision” and “timestamp” are present?
>  - the syntax grammar used in S3, P8 doesn’t make sense - use ABNF?
>  - In S3, P8: “the semicolons and the decimal point, if present, shall be replaced by underscores” - why are they not escaped?
>  - Example 1 seems semantically invalid?  e.g., "ietf-netconf-monitoring” is in "content-data" though not in "modules-state”.  Also, "module-set-id” is missing 
>  - It is unclear how the "inline-content-schema” feature could ever be used.  I.e., there are no protocol-accessible nodes in the module…
>  - "leaf-list inline-module" is "min-elements 1” and "ordered-by user”, but "leaf-list module” has neither (though it may be that ordering is irrelevant for simple-inline).
>  - "leaf-list inline-module” has "ordered-by user”, what does that mean in the context of, e.g., UC1?
>  - P4 in the “description” statement on line 134 in the YANG module makes no sense (grammatically incorrect).  Try breaking into multiple sentences.
>  - The last two sentences of the “description” statement on line 207 in the YANG module contradict each other.
> 
> 
> Structural issues:
> 
>  - Why isn’t the “Introduction” Section ‘1’ as is common in RFCs?
>  - the list in S3, P4 is missing symbols.
>  - the two lists in S3, P8 should be indented, or remove the leading symbol
>  - The list under "Metadata MUST include:” is missing symbols.
>  - The list under "Metadata SHOULD include:” is not indented.
>  - 3.2, Example 1: move the comment under "content-data” under "modules-state”.  Replace said comment with a new comment that says something like “This content-data only contains the data identified by the ‘content-schema’ node, despite the ‘module-state’ node indicating that the server implements other modules
>  - The three examples should be <section> of their own (e.g., 3.2.x)
>  - The “inline” choice node is generally confusing.  I can’t tell if it’s missing container called “inline” or if the two descendant nodes are poorly named.  In either case, it would be best to try to make it more readable.
> 
> 
> Editorial issues:
> 
>  - s/The term Server is/The term “server" is/
>  - s/not available/unavailable/
>  - s/Data is often needed already at/Data is often needed at/
>  - s/needed by groups that do not have a live running server available/needed when a live running server is unavailable/.
>  - s/ietf-yang- instance-data/"ietf-yang-instance-data”/
>  - remove the word “we” throughout: s/we/this document/?
>  - remove P3’s forward-reference to S3, P9?
>  - s/is actually implemented/is implemented/
>  - s/e.g., UC5 documenting diagnostic data/(e.g., UC5 [Section 2])/
>  - s/and state data set/and state data/
>  - s/Config=true/configuration (“config true”)/
>  - s/Config=false/operational state data (“config false"/
>  - OLD: mandatory, min-elements, require-instance=true, must and when
>    NEW: “mandatory", "min-elements", "require-instance true", “ ust”, and “when”
>  - OLD: If revision information inside the data set is present
>    NEW: If “revision" information is present inside the data set:
>  - OLD: If the leaf "name" is present in the instance data header, this MUST be used.
>    NEW: If the leaf "name" is present in the instance data header, its value should be used for the "instance-data-set-name”.
>  - s/items MAY also be used/items MAY be used/
>  - s/Metadata MUST include:/\nMetadata MUST include:/
>  - s/Content schema specification/Content schema specification (i.e., the "content-schema” node)/
>  - s/Do not include the content-schema,/Do not include the "content-schema” node”;/
>  - s/for "UC2 Preloading Data”/for UC2 [Section 2],/
>  - S3.1.1 P2 doesn’t makes sense to me (esp. the verdt ref, which likely should be removed or better explained)
>  - s/The same-schema-as-file leaf/The "same-schema-as-file" leaf/
>  - s/In Use Case 6/in UC6 [Section 2]/
>  - S3.2 P1: s/examples use/example uses/ and s/for better formatting//.   
>  - s/is based on "UC1, Documenting Server Capabilities”/exemplifies UC1 [Section 2]/
>  - s/(a shortened)/a/g
>  - For Ex1: s/uses the inline method/uses the “inline" method/
>  - s/- Use case 1, Documenting server capabilities/Exemplifying UC1/
>  - s/is based on "UC2, Preloading Default Configuration”/exemplifies UC2 [Section 2]/
>  - For Ex2: s/uses the inline method/uses the "simplified-inline" method/
>  - s/- Use case 2, Preloading access control data/Exemplifying UC2/
>  - s/is based on UC5 Storing diagnostics data/exemplifies UC5 [Section 2]/
>  - s/statistics about NETCONF/statistics about the NETCONF server/
>  - s/- UC5 Storing diagnostics data/Exemplifying UC5/
> 
> 
> Editorial issues inside the YANG module:
>  - "description" statement on line 74: rephrase to make more sense.
>  - :description" statement on line 92: so confusing.  Just write “The ‘revision' of the 'ietf-yang-instance-data’ module used to encode this 'instance-data-set’.”
>  - “description" statement on line 100: s/content schema/schema (i.e., YANG modules)/?
>  - “type string” statement on lines 109 and 131 are missing a “pattern" statement.
>  - “description" statement on line 110: should this be mostly the same as the description statement of line 134, sans the bit regarding features, deviations, etc.?
>  - line 152: s/ietf-yang-library@2019-01-04/revision "2019-01-04” of the "ietf-yang-library” module/?
>  - P2 in the “description" statements on lines 220 and 249: s/For instance data sets/Instance data sets/
>  - “description" statement on line 256: s/Modules/modules/ and s/content-schema-spec/‘content-schema’/
> 
> 
> YANG module formatting issues:
> 
>  - On line 19: s/  reference/reference/
>  - On line 21: s/}/  }/
>  - On line 235: s/pattern/  pattern/
> 
> 
> 
> Kent // shepherd
> 
> 
> 
> _______________________________________________
> netmod mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/netmod