Subject: [core] Review of draft-ietf-core-yang-cbor-15

From: Francesca Palombini <[email-masked]>
Date: Wed, 24 Feb 2021 12:41:22 +0000

Thanks for the work on this document. I have reviewed it and have some comments. Please handle these comments together with the Last Call comments.

Chairs: I believe it would make sense to request a review from YANG Doctors for this document. Let me know if you need help setting that up.

Thanks,
Francesca

==

4.4.  The 'list' and 'list' instance(s) - should this second 'list' be "subset of 'list'"

==

Section 4.6:

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

Why not BCP 14 SHALL?

==

* General question: about the SID values used throughout the document: as far as I can tell these are not allocated right now (and they will take some time to get registered as defined in core-sid document) It might be good to add a sentence about where the SID values from the examples come from.

==

Section 6.6 (and other sections that use tags defined by this document):

There is a new CBOR tag appearing in this section's example, tag 44. This is quite surprising without any text. Although the registration follows the template from RFC8949, I would have expected to see some descriptive text about the tag (and not only appearing in the sentence "The encoding MUST be enclosed by the enumeration CBOR tag as specified in Section 9.3." - note that section 9.3 does not specify how to use the tags).
This is valid for the following sections specifying new CBOR tags as well.

==

Section 6.7:

I find it hard to understand the example, without any indication on which bits are set. 

==

Nits

- expand acronyms on first use ( e.g. NETCONF, RPC, see https://www.rfc-editor.org/materials/abbrev.expansion.txt )

- move YANG SID terminology description before delta description.

Section 4.5

   module event-log {
     ...
     anydata last-event;                # SID 60123

Please close the parenthesis of this example