[Top] [All Lists]

Re: Working Group Last Call on draft-ietf-sieve-notify-05.txt

2006-12-14 13:39:49

Michael Haardt wrote:

Sorry for not responding earlier on this, but the end of the year is
always a real busy time.  I did not review the draft in depth _this time_,
but wanted to make at least a few comments on reading it quickly:

Thanks for the review!


  This document does not specify the notification
method but is expected that using existing instant messaging
I think the word "it" is missing before "is expected".

I've also deleted Zephyr and replaced Jabber with XMPP.

  infrastructure such as Zephyr, Jabber, or SMS messages will be

This sentence sounds wrong to me, but I am not a native speaker.

3.1.  Notify Action Syntax and Semantics

  Usage:  notify ":method" string
          [":from" string]
          [":importance" <"1" / "2" / "3">]
          [":options" string-list]
          [":message" string]
Ok, so the method is required now. Good! But why a tagged argument
instead of a positional argument? I don't mind it being one way or
another, I am just curious.  Do we have required tagged arguments
anywhere else?

I just didn't want to remove the tag everywhere in the document, as it might introduce editorial errors.
If people have a strong desire to remove the tag, I can remove it.

Optional tagged arguments can appear in any order, but they must appear before positional arguments. So if we switch to make the method argument positional, the usage statement needs to be modified to become:

Usage: notify [":from" string]
          [":importance" <"1" / "2" / "3">]
          [":options" string-list]
          [":message" string]

3.5.  Notify tag ":options"

  Each string in the options string list has the following syntax:
  "<optionname>=<value>" [[Alexey 3: Should we say something about
  implementation prefix for implementation specific options?  Something
  like "x-Vendor-zzz".  If we don't say it now, it might be too late to
  say it later.]]

Each method will specify used options, and adding anything, no matter
how it is named, is not going to be portable.  To me, dividing the name
space makes sense if having a registry for it, but I consider that to
be overkill for option strings.  We don't have a registry for envelope
key strings either, and I don't think we need one.

Yes, I don't think we are going to have very many envelope key strings.

Unless I hear some objections, I will take my comment out before requesting publication of the document.

3.8.  Requirements on notification methods specifications

  It is RECOMMENDED that a timestamp be included in the notification.

To me, that is not a requirement on the method. How about putting that
in section 3.6? Or do I get something completely wrong here and you
talk about a property of notification messages, not their message?
Mostly the latter.

If so, I suggest:

  If the notification method allows to transmit a timestamp outside
  the textual message, it is RECOMMENDED to include it.
How about:

  If the notification method is capable of transmitting a timestamp outside
  the textual message, it SHOULD include the timestamp.

But I am also thinking that implementations may include the timestamp in the textual message, if one is not specified. So maybe "outside the textual message" should be dropped.

7.  Security Considerations

  Notifications can result in loops and bounces.  In particular, a
  notification to an email address will not contain necessary Received
  header fields that might be otherwise used to prevent mail loops.
  All notification methods must take care to provide mechanisms for
  avoiding notification loops.

Did we decide on that yet? I vote to leave issues specific to "mailto"
out of this spec.
I think the security consideration is generally applicable to a wide range of notification mechanisms, but the second quoted sentence can be changed to start with "For example, ..."?

All in all, the draft is fine with me.  Despite my comments, I don't
object the last call as it is.
Great :-).