Subject: [Gen-art] Gen-ART LC/Telechat Review of draft-ietf-jcardcal-jcard-04

From: Ben Campbell <[email-masked]>
Date: Tue, 16 Jul 2013 17:27:31 -0500

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 wait for direction from your document shepherd
or AD before posting a new version of the draft.

Document:  draft-ietf-jcardcal-jcard-04
Reviewer:  Ben Campbell
Review Date:  2013-07-16
IETF LC End Date: 2013-07-18
IESG Telechat date: 2013-07-18

Note: This draft is on the IESG Telechat agenda on the same date as it completes IETF Last Call. Therefore, this review serves both purposes.

Summary: This draft is almost ready for publication as a proposed standard. There are a few minor issues and editorial nits that should be considered prior to publication.

Major issues:

-- None

Minor issues:

-- Section 1, design considerations:

You mention that the semantic results should survive round-tripping, but the order of fields might not. I gather there are other changes that might occur from the literal text, right? (e.g. Case changes, some optional parameter usage). If so, it might be worth clarifying that.

-- 3.1, 2nd paragraph:

I assume the removal of escaping means that one renders the escaped text, not simply removes it, right? Is that as simple as removing the escape characters in vCards? I assume this doesn't apply to any content-specific escaping inside vCard fields, e.g. URI escaping, right? If so, it might be worth mentioning.

-- 3.2.1.1:

What happens for future versions of vCard? Do you simply use the new version number, or would we need a new version of this spec? I suspect the latter. If true, it might be worth mentioning that, or somewhere early in the draft mention that this spec is for vCard 4.0 only.

-- sections 3.4.3 and 3.4.4:

Is the included ABNF a quotation of that in the referenced RFCs, or is it new and authoritative? If the former, it would be helpful to mention that in the text. (I note you did say that about the ABNF in the appendix).

-- 3.4.11:

If RFC6350 says that use of the timezone offset is NOT RECOMMENDED, can you elaborate on why it's included here? (I can guess it's because people may still use it in vCards since it was a "MUST NOT", but it would be good to say something to that effect in the text.)

Nits/editorial comments:

-- Section 1, paragraph 1:

Please expand JSON on first mention.

-- Section 1, paragraph 3:

This paragraph seems redundant to paragraph 1.

-- 1, paragraph 7: " Preserve the semantics of the vCard data."

Imperative form sentence is confusing in context, and inconsistent with surrounding text.

-- 1, paragraph 8:

Sentence Fragment.

-- 3.2, Last Paragraph: "... for a parser check the data type..."

Missing "to"?

-- 3.2.1.2, last paragraph before example:

Should the "iCalendar" reference be "vCard" instead?

-- 3.2.1.3, Last Paragraph:

RFCTODO? I gather in the IANA section, that may be a placeholder for "this RFC", but that doesn't seem to fit here.

-- 3.3.2: "Example 1:"

Other examples are not numbered.

-- 5:

More occurrences of RFCTODO

---

From: Jari Arkko <[email-masked]>
Date: Wed, 17 Jul 2013 07:01:28 +0200

Many thanks for your review, Ben! These reviews are very helpful for me to form an opinion about the draft in the IESG telechat. Authors, do you have thoughts on the issues that Ben raises here?

Jari

On Jul 17, 2013, at 12:27 AM, Ben Campbell <[email-masked]> wrote:

> _______________________________________________
> Gen-art mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/gen-art

---

From: Philipp Kewisch <[email-masked]>
Date: Mon, 22 Jul 2013 13:55:25 +0200

> You mention that the semantic results should survive round-tripping, but the order of fields might not. I gather there are other changes that might occur from the literal text, right? (e.g. Case ch…
Yes, thanks. I've added the case changes since even though upper-case is 
RECOMMENDED in rfc6350, it is indeed information that is lost. Could you 
elaborate which optional parameters you mean? If you mean the GROUP 
parameter, this parameter is to be removed when converting back into 
vCard, so there is no change to the original group construct.

I neither found nor recall any other examples where it would be 
necessary to add anything.

> I assume the removal of escaping means that one renders the escaped text, not simply removes it, right? Is that as simple as removing the escape characters in vCards? I assume this doesn't apply to…
I agree "removed" isn't the best word, I wasn't really happy with the 
alternatives though, maybe "unescape" will work out (not sure if that is 
a real word?). If not, then I'd go with your "render", although to me 
that always suggests it is actually displayed. I'm happy for more 
suggestions though:

OLD:
           When converting from vCard to jCard, lines MUST be unfolded 
first.
           Afterwards, vCard escaping MUST be removed.  The only 
escaping that
           may be applied is any escaping mandated by JSON (e.g., for 
control
           characters).

           The reverse order applies when converting from jCard to 
vCard. First,
           JSON escaping MUST be removed. Afterwards, vCard escaping MUST be
           applied. Finally, long lines SHOULD be folded as described in 
[RFC6350].

NEW:

           When converting from vCard to jCard, lines MUST be unfolded 
first.
           Afterwards, vCard escaping MUST be unescaped. The only 
escaping that
           may be applied is any escaping mandated by JSON (e.g., for 
control
           characters) and any content-specific escaping inside vCard 
values,
           e.g. URI escaping.

           The reverse order applies when converting from jCard to 
vCard. First,
           JSON escaping MUST be unescaped. Afterwards, vCard escaping 
MUST be
           applied. Finally, long lines SHOULD be folded as described in 
[RFC6350].
I'd love to hear from the WG here, but given vCard 5.0 can at least in 
theory be completely different, I think the correct thing to do would be 
to restrict this jCard document to 4.x and when 5.x comes around create 
a revision of the document. Of course the text could be changed so that 
its "valid until revised by another document", in case the changes in 
5.x are minor enough that no change in jCard is needed.

In any case, there should be a text change that any 4.x revision is 
allowed. For example, I recently read a suggestion for signed vCards 
that might use a version "4.0s".
> Is the included ABNF a quotation of that in the referenced RFCs, or is it new and authoritative? If the former, it would be helpful to mention that in the text. (I note you did say that about the A…
The ISO specs don't provide a direct BNF for any of their constructs. 
Instead they show examples in the form:
Basic format: YYYYMMDD Example: 19850412
Extended format: YYYY-MM-DD Example: 1985-04-12

The ABNF in jCard mimics this format, so it is not new, but given it 
matches I guess we could say its authoritative. Unless someone tells me 
otherwise (I'm still a bit uncertain when something can be called 
authoritative), I'll change it by changing the hangText "ABNF Schema:" 
to "ABNF Schema (authoritative):" in both sections.

There is no other vCard property that uses a utc-offset as a type, so 
this is just for the sake of an example. RFC6350 Section 6.5.1 has the 
details, it says utc-offset SHOULD NOT be used on a TZ property. The 
alternative would be to use an x-property for the example, i.e

["x-clock-offset", {}, "utc-offset", "-05:00"]

Do you think we should use this instead?

> Please expand JSON on first mention.
Doing this in the introduction since you reference Section 1, paragraph 
1. It already appears in the abstract, should I expand it there 
instead/in addition?

As the first mention uses "JSON-based", I've reworded these two paragraphs:

OLD:
    As certain similarities exist between vCard and the iCalendar data
    format [RFC5545], there is also an effort to define a JSON-based data
    format for calendar information called jCal [I-D.ietf-jcardcal-jcal]
    that parallels the format defined in this specification.

    The purpose of this specification is to define "jCard", a JSON format
    for vCard data.  One main advantage to using a JSON-based format as
    defined in [RFC4627] over the classic vCard format is easier
    processing for JavaScript based widgets and libraries, especially in
    the scope of web-based applications.

NEW:
    As certain similarities exist between vCard and the iCalendar data
    format [RFC5545], there is also an effort to define a JSON-based data
    format for calendar information called jCal [I-D.ietf-jcardcal-jcal]
    that parallels the format defined in this specification.  The term
    JSON describes the JavaScript Object Notation defined in [RFC4627].

    The purpose of this specification is to define "jCard", a JSON format
    for vCard data.  One main advantage to using a JSON-based format over
    the classic vCard format is easier processing for JavaScript based
    widgets and libraries, especially in the scope of web-based
    applications.

> This paragraph seems redundant to paragraph 1.
Yes, the duplicate is now removed. Sorry about that!
> Imperative form sentence is confusing in context, and inconsistent with surrounding text.
OLD:
       Preserve the semantics of the vCard data.  While a simple consumer
       can easily browse the data in jCard, a full understanding of vCard
       is still required in order to modify and/or fully comprehend the
       directory data.

NEW:
      The vCard data semantics are to be preserved, allowing a simple
       consumer to easily browse the data in jCard.  A full understanding
       of vCard is still required in order to modify and/or fully
       comprehend the directory data.

> Sentence Fragment.
The original intent was to make this a list of considerations, but as 
the first sentences are full I agree this makes it seem weird. How is 
this? I'm a little unsure about the "must not", but since this is just a 
consideration for the document and not an order for the consumer I think 
its ok with this casing:

OLD:
       Ability to handle many extensions to the underlying vCard
       specification without requiring an update to this document.
NEW:
       Extensions to the underlying vCard specification must not lead to
       requiring an update to jCard.

> -- 3.2, Last Paragraph: "... for a parser check the data type..."
Done

> Should the "iCalendar" reference be "vCard" instead?
Yes, copy/paste fail from jCal. Fixed.
> RFCTODO? I gather in the IANA section, that may be a placeholder for "this RFC", but that doesn't seem to fit here.
My intention was to keep the registration template as general as 
possible, RFCTODO indeed references "this RFC", I've defined an entity 
for it in the source. Do you want me to replace it with "this document" 
instead? I was thinking if the registration template is maybe copied out 
of the document and gathered somewhere, it would make sense to reference 
the document by its rfc number.

> Other examples are not numbered.
Fixed

> More occurrences of RFCTODO
Same reason, they appear in a registration template. I'm happy to 
replace with "this document" in case I'm told the registration template 
is never extracted from the document without any processing.

Thank you for the review!

---

From: Simon Perreault <[email-masked]>
Date: Mon, 22 Jul 2013 18:14:18 +0200

> that might use a version "4.0s".

I'd prefer "valid until revised by another document", which is
equivalent to not saying anything since that's how RFCs work. Therefore,
my suggestion would be to simply remove this sentence: "Similarly, the
value of the version property MUST be "4.0"."

Simon

---

From: Ben Campbell <[email-masked]>
Date: Mon, 22 Jul 2013 15:24:54 -0500

Thanks for the response. A few comments inline. I removed sections that don't seem to need further comment.

> On 7/17/13 12:27 AM, Ben Campbell wrote:

[...]

> I'd love to hear from the WG here, but given vCard 5.0 can at least in theory be completely different, I think the correct thing to do would be to restrict this jCard document to 4.x and when 5.x comes around create a revision of the document. Of course the text could be changed so that its "valid until revised by another document", in case the changes in 5.x are minor enough that no change in jCard is needed.
> 
> In any case, there should be a text change that any 4.x revision is allowed. For example, I recently read a suggestion for signed vCards that might use a version "4.0s".

Pending the opinions of the work group, I'm happy with that approach.

> The ISO specs don't provide a direct BNF for any of their constructs. Instead they show examples in the form:
> The ABNF in jCard mimics this format, so it is not new, but given it matches I guess we could say its authoritative. Unless someone tells me otherwise (I'm still a bit uncertain when something can be called authoritative), I'll change it by changing the hangText "ABNF Schema:" to "ABNF Schema (authoritative):" in both sections.

By "authoritative", I don't mean anything particularly formal. It's really a matter of where the normative text lies. So, for someone implementing _this_ draft, would you expect them to look at the included ABNF only, or is it a case of the ABNF being here for convenience but a careful implementor should  also look at the ISO specs?

oops, I think I meant to say "SHOULD NOT".

>> 
> There is no other vCard property that uses a utc-offset as a type, so this is just for the sake of an example. RFC6350 Section 6.5.1 has the details, it says utc-offset SHOULD NOT be used on a TZ property. The alternative would be to use an x-property for the example, i.e
> 

I don't have strong feelings either way, other than if you include a "NOT RECOMMENDED" example, it might be wise to put in a sentence or two giving the reasoning.

> Doing this in the introduction since you reference Section 1, paragraph 1. It already appears in the abstract, should I expand it there instead/in addition?

I think the general approach is that the abstract doesn't "count" for this purpose. That is, if you separated the abstract and the body into two different documents, they should both still make sense.

>    widgets and libraries, especially in the scope of web-based

That works for me.

[...]

> 
>> 
>> -- 1, paragraph 8:
>> 
>> Sentence Fragment.
>> 
> The original intent was to make this a list of considerations, but as the first sentences are full I agree this makes it seem weird. How is this? I'm a little unsure about the "must not", but since this is just a consideration for the document and not an order for the consumer I think its ok with this casing:
> 
> OLD:

WFM.

[...]

> My intention was to keep the registration template as general as possible, RFCTODO indeed references "this RFC", I've defined an entity for it in the source. Do you want me to replace it with "this document" instead? I was thinking if the registration template is maybe copied out of the document and gathered somewhere, it would make sense to reference the document by its rfc number.

By registration template, you are talking about the use in the IANA Considerations, right? My comment was about its use in sections 3.2.1.3 and 5. I think it makes sense in the IANA considerations (perhaps with a note to the RFC Editor to replace it with the actual RFC number), but "this document" would make more sense in 3.2.1.3 and 5.

[...]