Re: [apps-discuss] draft-wilde-xml-patch-08

[Tony Hansen <[e-mail]>]

This is a review of draft-wilde-xml-patch-08.

Document: draft-wilde-xml-patch-08.txt
Title: A Media Type for XML Patch Operations
Author: Erik Wilde
Reviewer: Tony Hansen
Review Date: March 25, 2014

Some minor issues are described below that should be considered. This 
document is otherwise ready to be published.

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

     Tony Hansen

Major issues: none

 >>>> Nit. Abstract. Consider changing this sentence as indicated.

    The XML Patch media type "application/xml-patch+xml" defines an XML
    document structure for expressing a sequence of patch operations that
    are applied to an XML document.
to
    The XML Patch media type "application/xml-patch+xml" defines an XML
    document structure for expressing a sequence of patch operations >>>to
    be<<< applied to an XML document.


 >>>>Nit. Abstract. This sentence needs to be split apart. I suggest 
just changing the comma after "RFC 5261" to a period.

    " The XML Patch document format's
    foundations are defined in RFC 5261, this specification defines a
    document format and a media type registration, so that XML Patch
    documents can be labeled with a media type, for example in HTTP
    conversations."


 >>>>Minor. Abstract.

Actually, on rereading the abstract, I'm thinking it would be better 
expressed like the following. The primary focus of the I-D is the 
document format itself. The media type registration is a secondary 
consideration. In the following, I tried not to change your points, just 
the ordering and emphasis.

    The XML Patch document format defines an XML
    document structure for expressing a sequence of patch operations to
    be applied to an XML document.  The XML Patch document format
    builds on the foundations defined in RFC 5261. This specification 
defines the
    also provides the media type registration "application/xml-patch+xml",
    to allow the use of XML Patch documents in, for example, HTTP 
conversations.



 >>>>Nit: More white space would be useful around the XML segments, to 
offset them more.

 >>>>Minor: I STILL feel that section 2.2 would read better if it shows 
a before and after example of applying the principle described there.

For example, after the second paragraph in section 2.2, add something 
like the following. (I think I got the conversion right below.)

vvvvvv ====== vvvvvv
For example, consider the patch example in RFC 5621 Appendix A.1. Adding 
an Element. The patch
is shown there as an XML diff document:

    <?xml version="1.0" encoding="UTF-8"?>
    <diff>
      <add sel="doc"><foo id="ert4773">This is a new child</foo></add>
    </diff>

With XML Patch, this would be converted to

    <?xml version="1.0" encoding="UTF-8"?>
    <p:patch xmlns="http://example.com/ns1" xmlns:p="urn:ietf:rfc:XXXX">
      <p:add sel="doc"><foo id="ert4773">This is a new child</foo></p:add>
    </p:patch>
^^^^^^ ====== ^^^^^^

 >>>>Nit: Appendix A

"It described" => "It describes"

 >>>>Nit: Appendix A.1

"XMPL Patch" => "XML Patch"

 >>>>Minor. Appendix C
There needs to be a pointer to the ABNF spec, RFC 5234, where things 
like DIGIT and DQUOTE are defined.
I did not review in detail the ABNF grammar itself.