Subject: Gen-ART review of draft-ietf-vcarddav-webdav-mkcol-05

From: "Spencer Dawkins" <[email-masked]>
Date: Fri, 7 Aug 2009 06:36:41 -0500

Hi, Cyrus,

I have been selected as the General Area Review Team (Gen-ART) reviewer for
this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Please wait for direction from your document shepherd or AD before posting a
new version of the draft.

Document: draft-ietf-vcarddav-webdav-mkcol-05
Reviewer: Spencer Dawkins
IETF LC End Date: 2009-08-17
Review Date: 2009-08-07
IESG Telechat date: (not known)

Summary: This document is almost ready for publication as a Proposed 
Standard. I have some minor comments (identified as "Spencer (minor)" in the 
following text, but nothing that should slow document approval. In 
particular, I'm pretty sure I identified an error in the title for Section 
3.5 below.

Comments identified as "Spencer (clarity)" are provided for editors, but are 
not part of the Gen-ART review.

Abstract

   This specification extends the Web Distributed Authoring and
   Versioning (WebDAV) MKCOL method to allow collections of arbitrary
   resourcetype to be created and to allow properties to be set at the
   same time.

Spencer (clarity): I know that RFC 4918 didn't provide an expansion for 
MKCOL until about page 46 (or maybe 25, in passing), but since MKCOL appears 
in the abstract for this document, it might be helpful to s/MKCOL/MKCOL 
("Make Collection")/ here. MKCOL is defined at first use in the 
Introduction, so that's fine - this is only a suggestion about the abstract.

3.  WebDAV extended MKCOL

   The WebDAV MKCOL request is extended to allow the inclusion of a
   request body.  The request body is an XML document containing a
   single DAV:mkcol XML element as the root element.  The Content-Type

Spencer (minor): if I'm reading this paragraph correctly, I'd suggest "The 
request body is an XML document that MUST contain a single DAV:mkcol XML 
element as the root element" here - the last sentence in this paragraph 
makes me think the requirement is normative, but it doesn't look normative 
to 2119 scanners :-)

   request header MUST be set appropriately for an XML body (e.g., set
   to "text/xml" or "application/xml").  XML-typed bodies for an MKCOL
   request that do not have DAV:mkcol as the root element are reserved
   for future usage.

   As per the PROPPATCH method ([RFC4918], Section 9.2), servers MUST
   process any DAV:set instructions in document order (an exception to
   the normal rule that ordering is irrelevant).  Instructions MUST
   either all be executed or none executed.  Thus, if any error occurs

Spencer (clarity): this sentence reads roughly to me, and it's 2119 
language, so needs to be clear. I suggest "The set of instructions MUST be 
executed atomically; either all are executed or none are executed".

   during processing, all executed instructions MUST be undone and a
   proper error result returned.  Failure to set a property value on the
   collection MUST result in a failure of the overall MKCOL request -
   i.e. the collection is not created.

3.5.  Example: Successful Extended MKCOL Request

Spencer (minor): Perhaps this should be s/Successful/Unsuccessful/? It's 
returning a 403/424 :D

4.  Replacing Existing MKxxx Methods

Spencer (clarity): Hmm. This section (and 4.1, and 4.1.1) aren't quite about 
REPLACING existing MKxxx methods, but more like "Providing MKxxx 
functionality using extended MKCOL". Would that be clearer? The current text 
in 4.1 sent me looking to see whether "replacement" meant "deprecation" (it 
doesn't, if I'm reading clearly), for example.

   One of the goals of this extension is to eliminate the need for other
   extensions to define their own variant of MKCOL to create the special
   collections they need.  This extension can be used to replace
   existing MKxxx methods in other extensions as detailed below.  If a
   server supports this extension and the other extension listed, then
   the server MUST support use of the extended MKCOL method to achieve
   the same result as the MKxxx method of the other extension.

---

From: Julian Reschke <[email-masked]>
Date: Mon, 17 Aug 2009 13:57:36 +0200

Spencer Dawkins wrote:
> ...
> Spencer (minor): if I'm reading this paragraph correctly, I'd suggest 
> "The request body is an XML document that MUST contain a single 
> DAV:mkcol XML element as the root element" here - the last sentence in 
> this paragraph makes me think the requirement is normative, but it 
> doesn't look normative to 2119 scanners :-)
> ...

-0.5

As far as I can tell, it is a myth that things can only be normative 
when using RFC 2119 keywords. As a matter of fact, RFC 2119 points out:

6. Guidance in the use of these Imperatives

    Imperatives of the type defined in this memo must be used with care
    and sparingly.  In particular, they MUST only be used where it is
    actually required for interoperation or to limit behavior which has
    potential for causing harm (e.g., limiting retransmisssions)  For
    example, they must not be used to try to impose a particular method
    on implementors where the method is not required for
    interoperability.

So I'd prefer document authors to be more conservative in using those 
terms...

BR, Julian

---

From: "Spencer Dawkins" <[email-masked]>
Date: Mon, 17 Aug 2009 08:09:07 -0400

Hi, Julian,

I agree with your point here (on how RFC 2119 works). I thought the document 
would be clearer with 2119 language here, but it should not be included if 
you aren't comfortable using it.

Thanks,

Spencer

>> "The request body is an XML document that MUST contain a single DAV:mkcol 
>> XML element as the root element" here - the last sentence in this 
>> paragraph makes me think the requirement is normative, but it doesn't 
>> look normative to 2119 scanners :-)
>> ...
>
> -0.5
>
> As far as I can tell, it is a myth that things can only be normative when 
> using RFC 2119 keywords. As a matter of fact, RFC 2119 points out:
>

---

From: Cyrus Daboo <[email-masked]>
Date: Tue, 18 Aug 2009 09:40:24 -0400

Hi Spencer,
Thanks for your review - I was on vacation last week so I am only getting 
around to replying now.

--On August 7, 2009 6:36:41 AM -0500 Spencer Dawkins 
<[email-masked]> wrote:

> Comments identified as "Spencer (clarity)" are provided for editors, but
> are not part of the Gen-ART review.
>
> Abstract
>
> MKCOL until about page 46 (or maybe 25, in passing), but since MKCOL
> appears in the abstract for this document, it might be helpful to
> s/MKCOL/MKCOL ("Make Collection")/ here. MKCOL is defined at first use in
> the Introduction, so that's fine - this is only a suggestion about the
> abstract.

Sure - seems reasonable. Change done in my working copy.

> look normative to 2119 scanners :-)

As per comments from Julian I won't make this change.

> language, so needs to be clear. I suggest "The set of instructions MUST
> be executed atomically; either all are executed or none are executed".

The text here is an exact copy of that in 4918. Strictly speaking all 
instructions are executed, it is just that some succeed and some fail. 
Perhaps better text is:

"If any one instruction fails to execute successfully, then all 
instructions MUST fail (i.e., either all succeed or all fail)".

> Spencer (minor): Perhaps this should be s/Successful/Unsuccessful/? It's

Fixed.

> Spencer (clarity): Hmm. This section (and 4.1, and 4.1.1) aren't quite
> about REPLACING existing MKxxx methods, but more like "Providing MKxxx
> functionality using extended MKCOL". Would that be clearer? The current
> text in 4.1 sent me looking to see whether "replacement" meant
> "deprecation" (it doesn't, if I'm reading clearly), for example.

Well ultimately I would like to see MKCALENDAR deprecated in favor of 
extended MKCOL - however, given the ever growing deployments of CalDAV that 
is probably not going to happen any time soon.

So, I have changed the title to:

"Using Extended MKCOL as an Alternative to MKxxx Methods"

and changed "replace" to "alternative for" in a couple of places.

-- 
Cyrus Daboo