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

["Joe Clarke (jclarke)" <[e-mail]>]

Fellow doctors, this was my first YANG doc review (and not even a YANG
module to boot).  I hope I did well, but would appreciate any spot
checks or advice.

Joe

On 3/16/21 15:36, Joe Clarke via Datatracker wrote:
> 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.
>
>
>
> _______________________________________________
> yang-doctors mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/yang-doctors
>