[Jcardcal] Genart LC review: draft-ietf-jcardcal-jcal-09

Robert Sparks <rjsparks@nostrum.com> Tue, 11 March 2014 18:55 UTC

Return-Path: <rjsparks@nostrum.com>
X-Original-To: jcardcal@ietfa.amsl.com
Delivered-To: jcardcal@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 658271A077D; Tue, 11 Mar 2014 11:55:22 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.447
X-Spam-Level:
X-Spam-Status: No, score=-2.447 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RP_MATCHES_RCVD=-0.547] autolearn=ham
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 rejLKfxDjsmO; Tue, 11 Mar 2014 11:55:21 -0700 (PDT)
Received: from nostrum.com (raven-v6.nostrum.com [IPv6:2001:470:d:1130::1]) by ietfa.amsl.com (Postfix) with ESMTP id F398D1A07B8; Tue, 11 Mar 2014 11:55:15 -0700 (PDT)
Received: from unnumerable.local (pool-173-71-10-88.dllstx.fios.verizon.net [173.71.10.88]) (authenticated bits=0) by nostrum.com (8.14.8/8.14.7) with ESMTP id s2BIt9aP073920 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=OK); Tue, 11 Mar 2014 13:55:09 -0500 (CDT) (envelope-from rjsparks@nostrum.com)
X-Authentication-Warning: raven.nostrum.com: Host pool-173-71-10-88.dllstx.fios.verizon.net [173.71.10.88] claimed to be unnumerable.local
Message-ID: <531F5C0D.5040903@nostrum.com>
Date: Tue, 11 Mar 2014 13:55:09 -0500
From: Robert Sparks <rjsparks@nostrum.com>
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.3.0
MIME-Version: 1.0
To: General Area Review Team <gen-art@ietf.org>, draft-ietf-jcardcal-jcal@tools.ietf.org, jcardcal@ietf.org
Content-Type: text/plain; charset="ISO-8859-1"; format="flowed"
Content-Transfer-Encoding: 7bit
Archived-At: http://mailarchive.ietf.org/arch/msg/jcardcal/DqBobymOGgw-vzLPp7BvYhQmgIU
X-Mailman-Approved-At: Tue, 11 Mar 2014 11:58:30 -0700
Subject: [Jcardcal] Genart LC review: draft-ietf-jcardcal-jcal-09
X-BeenThere: jcardcal@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: JSON data formats for vCard and iCalendar WG <jcardcal.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/jcardcal>, <mailto:jcardcal-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/jcardcal/>
List-Post: <mailto:jcardcal@ietf.org>
List-Help: <mailto:jcardcal-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/jcardcal>, <mailto:jcardcal-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 11 Mar 2014 18:55:22 -0000

I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at

<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-jcardcal-jcal-09
Reviewer: Robert Sparks
Review Date: 11Mar2014
IETF LC End Date: 12Mar2014
IESG Telechat date: 27Mar2014

Summary: Ready with nits

This is a solid document, and its development has left good artifacts 
showing a pattern of careful review.
(such as <http://trac.tools.ietf.org/wg/jcardcal/trac/report/6>).

Here are some nits to consider:

I agree with moving the reference to RFC4627 to normative, as already 
discussed.

Please consider adding a reference to clarify "JSON escaping" where it 
is mentioned in the 2nd paragraph of page 5.
Perhaps section 2.5 of rfc4627 would be a good reference?

The MUST in the third paragraph of 3.4.1.1 stuck out - is looks like a 
restatement of RFC5545 - that spec doesn't _allow_ anything but a 
semicolon for this particular separator. Would this be better written 
without 2119?
Perhaps: "When converting from jCal to iCalendar, be careful to use a 
semi-colon as the separator between the two values as required by RFC5545."

(This may be more than a nit): In the ABNF in section 3.6.5, where is 
the implementer supposed to go to find the definition of 'zone'? (Or the 
other production names?) I think _this_ chunk of ABNF (as opposed to 
that compiled in the appendix) is intended to be normative, yes? FWIW, 
it's not reflected in Appendix B.

I haven't extracted the BNF in appendix B and verified it, but it must 
fail - there is at least one typo. The expansion of param-multi includes 
"value-separtor" which should have been "value-separator".
Where is value-separator defined?

Just curious - has anyone tried converting a document from 
iCal->xCal->jCal->iCal? That might turn up some interesting corners that 
simple round-tripping might mask.

To try to save other reviewers some time, here are a couple of things I 
flagged that turned out to be non-issues:
* I was concerned with whether there would be issues with the forced 
conversion between upper and lower case. A little digging shows there is 
no issue - all the names this is done to are limited to the 
ascii-compatible characters.
* I verified that the syntax numbers with fractional parts is the same 
in both iCal in jCal. Specifically "4." is not valid in either grammar, 
so there is no need to discuss something like adding a 0 or remove the 
decimal point during conversion.