Subject: [siprec] Barry Leiba's No Objection on draft-ietf-siprec-metadata-20: (with COMMENT)

From: "Barry Leiba" <[email-masked]>
Date: Mon, 29 Feb 2016 13:01:00 -0800

Barry Leiba has entered the following ballot position for
draft-ietf-siprec-metadata-20: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-siprec-metadata/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I don't see how a reader can understand this without understanding what
an SRC, an SRS, an RS, and a CS are, and those are defined in RFC 6341. 
Why isn't 6341 a normative reference?

Please expand "AoR" on first use.  You might also cite 3261 there as
defining it.  (I find it a bit odd that there's no citation to 3261
except in the Security Considerations.)

You refer twice to "Unified Modelling Language(UML)": is there a
reference for UML?

On the other hand, while we're on references, I really don't think you
need any of the citations in Section 5.1.1 (and can remove their
associated references).  No one reading this needs to know the details of
what a URN is or how the ietf URN namespace is organized (just like you
don't have a reference for "URI", and don't need it).  I would just
simplify it to this and take out the three unnecessary references:

NEW
   The namespace URI for elements defined by this specification is
   the following URN:
   
      urn:ietf:params:xml:ns:recording:1
END

A really tiny point (which the RFC Editor might also bring up): In
Section 6.1.1 you refer to "a RS class" and "a RS object".  Unless that's
actually pronounced "Recording Session" when you read it, it should be
"an RS [class|object]".

There are a lot of missing articles ("the", "a", "an") throughout.  The
RFC Editor will fix these, but...

In Section 6.3.1, the definition of the "reason" attribute is slightly
confusing because "reason" is also an English word that can fit into the
sentences also.  So, here:

      The reason XML element has
      'protocol' as an attribute, which indicates the protocol from
      which the reason string is derived.

...I initially read "the reason XML element has 'protocol' as an
attribute..." differently to how you meant it.  I suggest making it
this:

      The 'reason' XML element has
      'protocol' as an attribute, which indicates the protocol from
      which the reason string is derived.

I also suggest going back through this and consistently quoting the words
when they refer to attribute or element names, to distinguish them from
just plain English ("the 'session' XML element", "the 'reason' XML
element", " 'protocol' attribute", and so on).  You could skip this for
the camel-cased items (such as "sipSessionID"), and the hyphenated ones
("group-ref"), because they can't be confused with plain words... but it
might be better to be consistent.

In Section 6.3.2, I don't like the two non-parallel lists at the end.  I
think the RFC Editor will fix these, so I'll just point them out and let
you decide what to do (or not).  (In contrast, the list in Section 6.5.2
is properly parallel.)

In Section 6.9, just checking here:

   As a security measure, the timestamp element SHOULD be included in
   all tuples unless the exact time of the status change cannot be
   determined.

Are there other reasons one might not include a timestamp element?  Or is
the fact that you can't determine the time the only one?  If the latter,
then it should be "MUST ... unless", rather than "SHOULD ... unless", and
I think that's what you mean.

---

From: "Ram Mohan R (rmohanr)" <[email-masked]>
Date: Tue, 1 Mar 2016 03:44:16 +0000

Hi Barry,

Thanks for your feedback. Please see inline

-----Original Message-----
From: Barry Leiba <[email-masked]>
Date: Tuesday, 1 March 2016 at 2:31 AM
To: The IESG <[email-masked]>
Cc: "[email-masked]"
<[email-masked]>, Brian Rosen <[email-masked]>,
"[email-masked]" <[email-masked]>, Brian Rosen
<[email-masked]>, "[email-masked]" <[email-masked]>
Subject: Barry Leiba's No Objection on draft-ietf-siprec-metadata-20:
(with COMMENT)

>https://datatracker.ietf.org/doc/draft-ietf-siprec-metadata/

we will make 6341 a normative reference.

>defining it.

Ok will do

>  (I find it a bit odd that there's no citation to 3261

I was looking at few other drafts. They seem to refer
http://www.omg.org/spec/UML/2.4/
I will do the same here.

>      urn:ietf:params:xml:ns:recording:1

Proposed text works for me.

>actually pronounced "Recording Session" when you read it, it should be

Will fix 

>RFC Editor will fix these, but...

I will read the document and fix any obvious ones I can identify.

>      which the reason string is derived.

I will fix with the proposed text.

>might be better to be consistent.

Agree. To be consistent I will use the single quote (like Œreason¹) for
all attributes and element names
to distinguish from plain english
>the fact that you can't determine the time the only one?

AFAIK the only case is where SRC cannot determine the time. Other wise
timestamp is included.
I will fix it with the following text:

 "As a security measure, the timestamp element MUST be included in
   all tuples unless the exact time of the status change cannot be
   determined.²

Regards,
Ram

> If the latter,

---

From: Barry Leiba <[email-masked]>
Date: Mon, 29 Feb 2016 22:46:37 -0500

Thanks for the quick response, Ram, and for addressing all my comments.

Barry