[core] Yangdoctors last call review of draft-ietf-core-yang-cbor-15

Joe Clarke via Datatracker <noreply@ietf.org> Tue, 16 March 2021 19:36 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: core@ietf.org
Delivered-To: core@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 67CD33A0CB7; Tue, 16 Mar 2021 12:36:21 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Joe Clarke via Datatracker <noreply@ietf.org>
To: <yang-doctors@ietf.org>
Cc: core@ietf.org, draft-ietf-core-yang-cbor.all@ietf.org, last-call@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 7.27.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <161592338136.13562.2421108714318351834@ietfa.amsl.com>
Reply-To: Joe Clarke <jclarke@cisco.com>
Date: Tue, 16 Mar 2021 12:36:21 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/core/Y8MoJC0OsdJjUFKN233nZMtkg3w>
Subject: [core] Yangdoctors last call review of draft-ietf-core-yang-cbor-15
X-BeenThere: core@ietf.org
X-Mailman-Version: 2.1.29
List-Id: "Constrained RESTful Environments \(CoRE\) Working Group list" <core.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/core>, <mailto:core-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/core/>
List-Post: <mailto:core@ietf.org>
List-Help: <mailto:core-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/core>, <mailto:core-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 16 Mar 2021 19:36:28 -0000

Reviewer: Joe Clarke
Review result: Almost Ready

I have been asked to review this document on behalf of yang-doctors.  Overall,
I found the document with its many examples to be quite readable and clear. 
However, I did find a few readable and typo issues and I have a few questions. 
See below.

===

Abstract:

s/notifications and yang-data/notifications and the yang-data/

===

Section 2:

Move the definition of YANG Schema iDentifier higher up in the list of
terminology so it's defined before you use it when discussing deltas.  This
should likely be first in the list.

===

Section 3

s/When schema node are serialized/When schema nodes are serialized/

===

Section 3.3

s/identifiers as string, similar/identifiers as strings, similar/

===

Section 4.1

In other examples you include the associated type definition inline.  You don't
do that with inet:domain-name.  In fact, you _do_ include the domain-name
typedef in Section 4.3, but I think it should move up here as well to aid in
readability.

===

Section 4.2.1

s/In the case of an 'notification content'/In the case of a 'notification
content'/

===

Section 4.2.2

You duplicate the YANG snippet here that you included in the overarching
Section 4.2.  I don't think both are needed.  Probably best to drop this here
since you didn't include it in 4.2.1.

===

Section 4.4

You use the term "list instance" to mean what I think is better stated as "list
entry".  The latter is clearer with respect to an element within a list vs. the
instance of a list itself.  You use "list instance" in Section 3 as well (and
in other places in the document) where I think "list entry" would be clearer.

===

Section 4.4.1

I think documenting the true/false value of the primitives here (noted in the
CBOR encoding) would aid in clarity.  For example, "primitive(20) [false]".

===

Section 4.5.1

I'm being pedantic here, but ahead of the { 60123 : { ... example, you usually
state "CBOR diagnostic output".  You don't here, though.  I think you should
add it.

===

Section 4.6

Concerning text "anyxml value MAY contain CBOR data items tagged with one of
the tags listed in Section 9.3, these tags shall be supported.":

This sentence fragment makes no sense.  Did you mean something along the lines
of: "the tags listed in Section 9.3 SHALL be supported"?

===

Section 5

s/Just like YANG containers, yang-data/Just like YANG containers, the yang-data/

===

Section 6.7

s/same example yang definition, but this/same example YANG definition, but this/

s/byte string MUST instead by encoded/byte string MUST instead be encoded/

===

Section 6.13.1

It isn't clear to me how a YANG list with multiple keys or a YANG list with no
keys would be encoded in CBOR.  I think examples and some clarifying text would
help.