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

Kent Watsen <kent+ietf@watsen.net> Mon, 30 March 2020 19:22 UTC

Return-Path: <010001712ce4c5fe-04a059c3-ced3-4e6d-8389-5dd7c1257ac2-000000@amazonses.watsen.net>
X-Original-To: netmod@ietfa.amsl.com
Delivered-To: netmod@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id A258F3A0F27 for <netmod@ietfa.amsl.com>; Mon, 30 Mar 2020 12:22:05 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: 0.002
X-Spam-Level:
X-Spam-Status: No, score=0.002 tagged_above=-999 required=5 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, SPF_HELO_NONE=0.001, SPF_NONE=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=amazonses.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7whLzB0rxZH8 for <netmod@ietfa.amsl.com>; Mon, 30 Mar 2020 12:22:04 -0700 (PDT)
Received: from a48-92.smtp-out.amazonses.com (a48-92.smtp-out.amazonses.com [54.240.48.92]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 2519C3A0F23 for <netmod@ietf.org>; Mon, 30 Mar 2020 12:22:04 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=224i4yxa5dv7c2xz3womw6peuasteono; d=amazonses.com; t=1585596122; h=From:Content-Type:Content-Transfer-Encoding:Mime-Version:Subject:Message-Id:Date:To:Feedback-ID; bh=n5czoWSeI7mq++vP5Ckavv3eRitwf1G66/kQ3nzc7b4=; b=Rj4yudRK9pQMETn5xQbx4GMCgjuGl9lRRVXVGdYt0RKjHR+GlQqBbOtNBYxviIMd H/guxw/+bVO5+vGlxeehgmY1PAWUzZPgO1fET+afPH0p/zc6ykdRFZWVAc8g+84vWgk S7DEe5L9Klllz/rdGD9DC2AH1QjTflgY6VvGFaYk=
From: Kent Watsen <kent+ietf@watsen.net>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\))
Message-ID: <010001712ce4c5fe-04a059c3-ced3-4e6d-8389-5dd7c1257ac2-000000@email.amazonses.com>
Date: Mon, 30 Mar 2020 19:22:02 +0000
To: "netmod@ietf.org" <netmod@ietf.org>
X-Mailer: Apple Mail (2.3445.104.11)
X-SES-Outgoing: 2020.03.30-54.240.48.92
Feedback-ID: 1.us-east-1.DKmIRZFhhsBhtmFMNikgwZUWVrODEw9qVcPhqJEI2DA=:AmazonSES
Archived-At: <https://mailarchive.ietf.org/arch/msg/netmod/kLLWzTWoOg374jGSaBjXAt4Hx1I>
Subject: [netmod] Shepherd review on draft-ietf-netmod-yang-instance-file-format-10
X-BeenThere: netmod@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: NETMOD WG list <netmod.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netmod>, <mailto:netmod-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netmod/>
List-Post: <mailto:netmod@ietf.org>
List-Help: <mailto:netmod-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netmod>, <mailto:netmod-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 30 Mar 2020 19:22:06 -0000

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