Subject: [apps-discuss] apps-team review of draft-ietf-ecrit-lost-servicelistboundary-03

From: Joshua Bell <[email-masked]>
Date: Wed, 7 Jul 2010 12:29:30 -0700

Richard Barnes (ECRIT co-chair) requested a review
of draft-ietf-ecrit-lost-servicelistboundary-03

I have been selected as the Applications Area Review Team reviewer for this
draft (for background on apps-review, please see
http://www.apps.ietf.org/content/applications-area-review-team).
Please resolve these comments along with any other Last Call comments you
may receive. Please wait for direction from your document shepherd or AD
before posting a new version of the draft.

Document: draft-ietf-ecrit-lost-servicelistboundary-03
Reviewer: Joshua Bell
Review Date: 2010-07-07

Summary:

This draft is not ready for publication as a Proposed Standard;
inconsistencies in the XML examples and normative RelaxNG schema must be
addressed before publication.

Major Issues:

The Relax NG schema in section 5.1 appears to be invalid. (Caveat: I’m new
to RelaxNG) Validation of samples was attempted using xmllint ("using libxml
version 20627"), and using lost.rng from
http://www.tschofenig.priv.at/svn/draft-ietf-ecrit-lost/RelaxNG/lost.rngreferenced
in RFC5222
<ref name="slb:serviceListBoundaryResponse"/>

The grammar element of the schema must specify xmlns="
http://relaxng.org/ns/structure/1.0" rather
than xmlns="urn:ietf:params:xml:ns:lost1"; it should target the lost
namespace via the ns attribute i.e.:

<grammar ns="urn:ietf:params:xml:ns:lost1" xmlns="
http://relaxng.org/ns/structure/1.0
" xmlns:slb="urn:ietf:params:xml:ns:lost1:slb">

Per http://www.relaxng.org/spec-20011203.html the value of the name
attribute of a ref element must be a NCName, so the use of an XML namespace
prefix is invalid in ref:

<ref name="slb:serviceListBoundary"/>

A definition for an element named serviceListBoundary itself is not defined;
this is used in examples  (e.g. Section 3.1, first response) but not in the
RelaxNG Schema. The schema defines:

          <choice>
             <ref name="slb:serviceListBoundaryResponse"/>
             <ref name="slb:serviceListBoundaryReference"/>
           </choice>

I suspect that usage of serviceListBoundary (without a suffix) is an older
construction that was replaced by serviceListBoundaryResponse and
serviceListBoundaryReference when reference responses were added.

While the intent of the schema and examples are clear, I believe a further
review must be performed after these issues are corrected, the schema can be
tested and the examples can be validated.

Minor Issues:

Section 3.3

"The server may return... " - should be MAY
"...but has to use at least one ..." - should be MUST

Section 3.4.1

Much of this section is overly casual and thus confusingly worded; while
apparently not normative, they distract from the readability of the
document.

* "Probably for some countries..."
* "So a reasonable trade-off..."
* "So when moving..."
...and generally in the draft, any sentence with "has to" could stand to be
reworded for clarity, even if not normative

Section 3.4.2

".. the client must perform ..." - is this a MUST in the RFC 2119 sense? If
so, capitalize and make it clear that this is normative. Otherwise, reword.

Nits:

Section 3.4.1
* Typo ("disrict" --> "district").

---

From: Karl Heinz Wolf <[email-masked]>
Date: Thu, 8 Jul 2010 12:16:50 +0200

Joshua,
 
thank you for your detailed review - I appreaciate your comments.
Right now I'm struggeling with getting the namespace issue correct. How
do I reference to the xmlns:slb="urn:ietf:params:xml:ns:lost1:slb"
namespace, when I cannot use slb: in name attribute of the ref element?
Your help would be appricated!
Thank you,
Karl

________________________________

	Von: Joshua Bell [mailto:[email-masked]] 
	Gesendet: Mittwoch, 07. Juli 2010 21:30
	An: [email-masked];
[email-masked]; Marc Linsner;
Richard Barnes
	Betreff: apps-team review of
draft-ietf-ecrit-lost-servicelistboundary-03

	Richard Barnes (ECRIT co-chair) requested a review of
draft-ietf-ecrit-lost-servicelistboundary-03

	I have been selected as the Applications Area Review Team
reviewer for this draft (for background on apps-review, please see
http://www.apps.ietf.org/content/applications-area-review-team).
	Please resolve these comments along with any other Last Call
comments you may receive. Please wait for direction from your document
shepherd or AD before posting a new version of the draft.

	Summary: 

	Major Issues:

	The Relax NG schema in section 5.1 appears to be invalid.
(Caveat: I'm new to RelaxNG) Validation of samples was attempted using
xmllint ("using libxml version 20627"), and using lost.rng from
http://www.tschofenig.priv.at/svn/draft-ietf-ecrit-lost/RelaxNG/lost.rng
referenced in RFC5222
	<ref name="slb:serviceListBoundaryResponse"/>

	The grammar element of the schema must specify
xmlns="http://relaxng.org/ns/structure/1.0" rather than
xmlns="urn:ietf:params:xml:ns:lost1"; it should target the lost
namespace via the ns attribute i.e.: 

	<grammar ns="urn:ietf:params:xml:ns:lost1"
xmlns="http://relaxng.org/ns/structure/1.0"
xmlns:slb="urn:ietf:params:xml:ns:lost1:slb">

	Per http://www.relaxng.org/spec-20011203.html the value of the
name attribute of a ref element must be a NCName, so the use of an XML
namespace prefix is invalid in ref:

	A definition for an element named serviceListBoundary itself is
not defined; this is used in examples  (e.g. Section 3.1, first
response) but not in the RelaxNG Schema. The schema defines:

	          <choice>

	I suspect that usage of serviceListBoundary (without a suffix)
is an older construction that was replaced by
serviceListBoundaryResponse and serviceListBoundaryReference when
reference responses were added.

	While the intent of the schema and examples are clear, I believe
a further review must be performed after these issues are corrected, the
schema can be tested and the examples can be validated.

	Minor Issues: 

	Section 3.3

	Section 3.4.1

	Much of this section is overly casual and thus confusingly
worded; while apparently not normative, they distract from the
readability of the document. 

	* "Probably for some countries..."
	* "So a reasonable trade-off..."
	* "So when moving..."
	...and generally in the draft, any sentence with "has to" could
stand to be reworded for clarity, even if not normative

	Section 3.4.2

	".. the client must perform ..." - is this a MUST in the RFC
2119 sense? If so, capitalize and make it clear that this is normative.
Otherwise, reword.

	Nits: 

	Section 3.4.1