Ines, thanks for your review. Carsten, thanks for addressing the review comments. I have entered a No Objection ballot.
Alissa > On Nov 6, 2018, at 6:51 PM, Carsten Bormann <[email protected]> wrote: > > Hi Ines, > > Thank you very much for the review and the slight embarrassment it causes to > the authors… > > Comment 3 below we will pick up in the WG meeting today; I’ll report about > that later. > The other comments are now addressed in the editor’s copy: > > https://github.com/cbor-wg/cddl/commits/master > (Individual commits referenced below; the CI build is failing right now > because of a weird connectivity issue.) > > We plan to submit the fixes as part of -06 early next week. > > Grüße, Carsten > > > >> On Sep 29, 2018, at 05:02, Ines Robles <[email protected]> >> wrote: >> >> Reviewer: Ines Robles >> Review result: Ready with Nits >> >> I am the assigned Gen-ART reviewer for this draft. The General Area >> Review Team (Gen-ART) reviews all IETF documents being processed >> by the IESG for the IETF Chair. Please treat these comments just >> like any other last call comments. >> >> For more information, please see the FAQ at >> >> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. >> >> Document: draft-ietf-cbor-cddl-05 >> Reviewer: Ines Robles >> Review Date: 2018-09-28 >> IETF LC End Date: 2018-10-04 >> IESG Telechat date: Not scheduled for a telechat >> >> Summary: >> >> I believe the draft is technically good. This document is well written and >> clear to understand. >> >> The document proposes a notational convention named Concise data definition >> language (CDDL) to express Concise Binary Object Representation (CBOR) data >> structures. Its main goal is to provide an easy and unambiguous way to >> express >> structures for protocol messages and data formats that use CBOR or JSON. >> >> I have some minor questions. The document presents nits that should be solved >> before publication. >> >> Major issues: Not found. >> >> Minor issues: Not found. >> >> Nits/editorial comments: >> >> 1)-Abstract: It would be nice to expand CBOR the first time that it is >> mentioned. > > Indeed. Fixed in 88e9a00 > >> 2)-Section 2: Page 8, You mention anonymous group. >> Does it would be a good definition for an anonymous group?: “Lists of >> group_entries_ (between curly braces)that can be assigned to any type of >> composition (arrays, maps)” > > The groups are anonymous because they are used right away in a pair of curly > braces (for a map) or square brackets (for an array). > The examples in this section all happen to be in braces. > I’m not sure that “anonymous group” needs to be defined as a separate term, > though, because it is just that: a group that is anonymous. > >> 3)- Section 2.2.2.1 - Page 11: the ordering I think is ascendant, right? So, >> maybe, would it be nice to add “natural ordering relationship”? question: is >> the following valid?: >> >> Take-off = { >> takeoffcounting= 10..0 ; Is it valid? Is it applicable? >> } > > The type constituted by the range does not have any ordering relationship > (except that implied by numbering in general). > The example you give would not make a difference from saying 0..10 — in both > cases, the eleven numbers 0 to 10 constitute the type. > > It is probably not so good that the current text seems to leave open whether > 10..0 means the same as 0..10. > > (There is no obvious right answer, so we will discuss this in the CBOR WG > meeting today and come back.) > >> 4)- In this paragraph: “...When using a name as the left hand side of a range >> operator, use spacing as in >> "min .. max" to separate off the range operator...." >> => Question: Is “min .. max" equivalent to "min” .. “max"? > > Sorry for leading the reader into quoting hell. > The quotes here are generated by XML2RFC to delimit a code span, i.e., they > identify the example. > Maybe we should make small figures out of these (ignoring that some people > don’t like sentences running over figures). > > Fixed in a530564 > >> >> 5)- There are some mismatches between Appendix B and Appendix C: >> >> Appendix B: cddl = S 1*(rule S) >> Appendix C: cddl = S 1*rule >> ------------------------------------------------------------ >> >> Appendix B: >> rule = typename [genericparm] S assignt S type >> / groupname [genericparm] S assigng S grpent >> >> Appendix C: >> rule = typename [genericparm] S assignt S type S >> / groupname [genericparm] S assigng S grpent S >> ------------------------------------------------------------ >> >> Appendix B: type = type1 *(S "/" S type1) >> Appendix C: type = type1 S *("/" S type1 S) >> >> ------------------------------------------------------------ >> Appendix B: group = grpchoice *(S "//" S grpchoice) >> Appendix C: group = grpchoice S *("//" S grpchoice S) >> >> ------------------------------------------------------------ >> Appendix B: grpchoice = *(grpent optcom) >> Appendix C: grpchoice = *grpent >> >> ------------------------------------------------------------ >> >> Appendix B: grpent = [occur S] [memberkey S] type >> Appendix C: grpent = [occur S] [memberkey S] type optcom >> >> ------------------------------------------------------------ >> >> Appendix B: / [occur S] groupname [genericarg] ; preempted by above >> Appendix C: / [occur S] groupname [genericarg] optcom ; preempted by >> above >> >> ------------------------------------------------------------ >> >> Appendix B: / [occur S] "(" S group S ")" >> Appendix C: / [occur S] “(" S group S ")" optcom > > This is the embarrassing part. We recently cleaned up the whitespace > processing in the ABNF a bit, and I was sure all the changes made it to the > matching appendix. D’oh. Another case of “a feature isn’t there if it isn’t > checked by continuous integration checking”. > > Fixed in bfdecd8 > >> >> 6)- It would be nice to expand I-JSON to (“Internet JSON") in page 54 > > Fixed in cc43317 > > Grüße, Carsten > > _______________________________________________ > Gen-art mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/gen-art _______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
