Re: [Gen-art] Genart last call review of draft-ietf-cbor-cddl-05

Alissa Cooper <alissa@cooperw.in> Mon, 19 November 2018 19:40 UTC

Return-Path: <alissa@cooperw.in>
X-Original-To: gen-art@ietfa.amsl.com
Delivered-To: gen-art@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 23D7D12F1AB; Mon, 19 Nov 2018 11:40:44 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.701
X-Spam-Level:
X-Spam-Status: No, score=-2.701 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=cooperw.in header.b=hsGONsOq; dkim=pass (2048-bit key) header.d=messagingengine.com header.b=wbDLOubo
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 MMIPbK8XOm0L; Mon, 19 Nov 2018 11:40:42 -0800 (PST)
Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 227CF1292F1; Mon, 19 Nov 2018 11:40:42 -0800 (PST)
Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailout.west.internal (Postfix) with ESMTP id 330C1A33; Mon, 19 Nov 2018 14:40:41 -0500 (EST)
Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Mon, 19 Nov 2018 14:40:41 -0500
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cooperw.in; h= content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; s=fm1; bh=s WeH8hCY6zRgMQbF4CTJ5GkZVffMqjeU8myUKET1tAc=; b=hsGONsOqWKsVkMj5h WcDKvJ8lwrK/+XD5MbAPgdaSxBLrbqgAgmmcofgm9RB9C/2SY+mWL/Q3EMFoc6bA zGpaTs9FyKCYi8vNJEuvaWQdnjmoZCbcNxow6t7zPO1Txiym4tLA0f8GLi8BfWVu DByo6QLVoeWHHMffPQTewguPT7oF+FOoc4g7cguBnjQIswVheDJx6fYM9Zek5EUL XA9rRayNMBxpyMod/2QuWllGruWUF0HkjhkkmOamdorE/NXpv4kWnhFl10RM2c9y gudP0GXnZcCKYlNG5fuvY7RziyPy6HUOsuYB4vpBDShM+crf80a8INkiWNMC1eo1 E6wfg==
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=sWeH8hCY6zRgMQbF4CTJ5GkZVffMqjeU8myUKET1t Ac=; b=wbDLOubo5pZbgGKLTjHevbxEFLdnwNGMFKev29WDTbZTg9PVrXp9ThZHZ AkL+ljYKwudx89bQSEKx/kYgZ6v71gYtLXqd8iFYf+NHgfCVehMl+DP8gAL1+Y0e jOBIWfrYq4wOWxitZ2WeLie23pQq48QyAayjet97ejEoGqP84OYEtMv3GPudTN0R /jbZEVhghQGXIurJjb/f1g7FupCZVUkuf5NKINirP2NPGnpHqA7S6jxxhRrvISqz R/GMrYHsVZpQDZ7ye7/Tgxb91TUK6giwlBWE8BRjRUmczZrnhVeHhL4oVGeL5/PG EcXkBF7qdn123DO0df2OynHZf6gxg==
X-ME-Sender: <xms:uBHzW7a4fR2dpGRb070kBsLiMmxsDCF6LImO6tygvi2tvv32ENDJ0Q>
X-ME-Proxy: <xmx:uBHzW3HhRbjdzeIhf0jFBWCVWfZZ-E2MEVu6EwSC8j6MmsKyJ_govw> <xmx:uBHzWzieQ-AWNkJwd-gmUsD3czeQrQEg_Ejs4cvjm4ryrBMb8ab0jA> <xmx:uBHzW70YwmXixQkIyO5fCWVA8V4LNtpW3HJeMMRgsQ8flL18GTcVgQ> <xmx:uBHzW-ePazJtTTwWGAFyVX8O6Z1EZxE_ZKKncoFBa-taeXzeQEUmiQ> <xmx:uBHzW5wMuEgoxs42SjCjl3j8e1Vv5N6M2Fk-A75pBusUeHdIqxCHEw> <xmx:uBHzW8IIbR-1LQjyv7uWDVJcaoJk_huS7bedqyJcR1DIoBt3X0IpLQ>
Received: from rtp-alcoop-nitro5.cisco.com (unknown [173.38.117.73]) by mail.messagingengine.com (Postfix) with ESMTPA id BB87A10331; Mon, 19 Nov 2018 14:40:39 -0500 (EST)
Content-Type: text/plain; charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\))
From: Alissa Cooper <alissa@cooperw.in>
In-Reply-To: <10456266-70D0-43FD-8FB2-00A5EF01A6A4@tzi.org>
Date: Mon, 19 Nov 2018 14:40:38 -0500
Cc: cbor@ietf.org, General Area Review Team <gen-art@ietf.org>, draft-ietf-cbor-cddl.all@ietf.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <169C8F84-81B9-414A-9123-F3CD69A49B77@cooperw.in>
References: <153817214335.26462.1312093989723950989@ietfa.amsl.com> <10456266-70D0-43FD-8FB2-00A5EF01A6A4@tzi.org>
To: Carsten Bormann <cabo@tzi.org>, Ines Robles <mariainesrobles@googlemail.com>
X-Mailer: Apple Mail (2.3445.9.1)
Archived-At: <https://mailarchive.ietf.org/arch/msg/gen-art/qzZerlH4XsOYGZCbju3BwE1SlP4>
Subject: Re: [Gen-art] Genart last call review of draft-ietf-cbor-cddl-05
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 19 Nov 2018 19:40:44 -0000

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 <cabo@tzi.org> 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 <mariainesrobles@googlemail.com> 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
> Gen-art@ietf.org
> https://www.ietf.org/mailman/listinfo/gen-art