Re: [sieve] WGLC: draft-ietf-sieve-imap-sieve-04.txt

[Aaron Stone <[e-mail]>]

I have reviewed the changes in -05, and recommend this document for publication.

Cheers,
Aaron

On Fri, Jun 15, 2012 at 7:22 PM, Aaron Stone <[e-mail]> wrote:
> Thanks! No sweat on the comments not taken :)
>
> On Fri, Jun 15, 2012 at 6:50 PM, Barry Leiba <[e-mail]> wrote:
>> Thanks, Aaron, for the review.
>>
>>> RFC 5464 says:
>>>   All entries MUST have either "/shared" or "/private" as a prefix.
>>>   Entry names are case-insensitive.
>>>
>>> I think that means our metadata key should be
>>> "/shared/imapsieve/script".
>>
>> Well, it doesn't mean that it couldn't be, say,
>> "/shared/IMAPSieve/Script".  It says it's insensitive, so I can put it
>> in any case I like.  But as long as I have to change it to add the
>> prefix (the text was written when there was no prefix and "value.xxx"
>> was used), I also changed it to be all lower case.  Why not?
>>
>>> Section 2.1: feels like there should be a MUST in here somewhere?
>>>
>>> OLD
>>>   The corresponding Sieve implementation uses the Sieve capability
>>>   string "imapsieve", and Sieve scripts that depend upon the IMAP
>>>   events MUST include that string in their "required" lists.
>>> NEW
>>>   The corresponding Sieve implementation MUST provide the "imapsieve"
>>>   capability string (see [RFC5228] Section 6.1), and Sieve scripts that depend
>>>   upon IMAP events MUST include that string in their "required" lists.
>>
>> Absolutely not: that would make this the only Sieve extension to say
>> that.  All the extensions I've looked at (Vacation, Notify,
>> Editheader, Ihave, MIME-loop, Reject) do NOT use 2119 language when
>> they talk about the capability string.
>
> OK.
>
>>> Section 2.2: Add a MUST regarding individual script execution, because
>>> I missed this requirement the first two times I read through it:
>>> OLD
>>>   If more than one message is affected at the same time, each message
>>>   triggers the execution of a Sieve script separately.  The scripts MAY
>>>   be run in parallel.
>>> NEW
>>>   If more than one message is affected at the same time, each message
>>>   MUST trigger execution of the Sieve script separately.  The scripts MAY
>>>   be run in parallel.
>>
>> No.  If you're only taking text to be normative if it has a 2119
>> keyword in it, you need to read documents differently.  "This is how
>> the protocol works" statements don't need to have "MUST" all over
>> them, and I see no need for a MUST here.
>
> OK.
>
>>> Section 2.3.1: Convert from prose to list, and rephrase to include a
>>> lot of MUSTs
>>> OLD
>>>   When an applicable event occurs on an IMAP mailbox, if there is an
>>>   IMAP metadata entry named "/IMAPSieve/Script" for the mailbox, that
>>>   entry is used.  If there is not, but there is an IMAP metadata entry
>>>   named "/IMAPSieve/Script" for the server, that entry is used
>>>   (providing a way to define a global script for all mailboxes on a
>>>   server).  If neither entry exists, then no script will be invoked.
>>> NEW
>>>   When an applicable event occurs, the IMAP server MUST inspect
>>>   the following metadata entries, in order, to determine which Sieve
>>>   script to invoke:
>>>   o "/IMAPSieve/Script" for the target mailbox,
>>>   o "/IMAPSieve/Script" for the server.
>>>   If neither entry exists, then no script will be invoked.
>>
>> Same as above.  The original text is fine.  I dislike the list format
>> here, and see no reason for a MUST.
>
> OK.
>
>>> MINOR: I just realized that there might be confusion over an "empty"
>>> script name. I propose that a global server script, and a mailbox
>>> script with no value, means "run this script for every mailbox, except
>>> this one". If that's OK with you, I propose this text immediately
>>> following the text above:
>>> NEW
>>>    If the script name is an empty string, the server MUST NOT
>>>    inspect any further metadata entries (this allows a user to specify
>>>    a global server script, but disable it for specific mailboxes).
>>
>> No reason to tell it not to inspect further, because we've already
>> said that if it finds a mailbox entry, it has what it needs.  But I
>> like the idea of saying that an empty entry value means that no script
>> is run, so I'll add that.  New paragraph 3:
>>
>>                If a "/shared/imapsieve/script" metadata entry was selected
>>                above, its value is used as the name of the Sieve script
>>                that will be invoked in response to the IMAP event.
>>                If the value is empty, then no script is run.
>
> I dig your subtle style.
>
>> I'll post a new version after I send this.
>>
>> Barry