Subject: Working Group Last Call: draft-ietf-sieve-3431bis-01.txt (relational test)

From: Alexey Melnikov <[email-masked]>
Date: Tue, 06 Sep 2005 11:28:20 +0100

I would like to draw your attention to the following draft:

<http://www.ietf.org/internet-drafts/draft-ietf-sieve-3431bis-01.txt>

A two week working group last call of this document starts today on 
September 6th 2005 and ends on Tuesday, September 20th 2005 at 6 pm EST.

Please review this document and send issues to the list or direct to the 
author(s).

NB If you do review this document, but have no issues or comments, 
please send both co-chairs (or the list) an email to indicate you have 
looked at it. This will allow the chairs to gauge the scope of reviews 
of this document to aid in our determination on whether to send it to 
the IESG. Thanks.

--
Alexey Melnikov
SIEVE WG Co-chair

---

From: Michael Haardt <[email-masked]>
Date: Thu, 8 Sep 2005 14:37:11 +0200

> Please review this document and send issues to the list or direct to the 

I have no serious issues, but some minor ones:

I miss a list of changes.  Aren't there any? I miss a table of contents,
too.

I miss a section "Interaction with Other Sieve Actions".  Obviously,
"relational" is only incompatible with comparators that return no
sort information.  There are no incompatibilities with any actions
and implicit keep is not touched, but I would prefer not having to
read that between the lines.

Section 2:

   The capability string associated with extension defined in this
   document is "relational".

I think this does not belong to "Conventions used in this document",
but to its own section "Capability Identifier".

Section 4.1:

   The VALUE match type may be used with any comparator which returns
   sort information.

And which comparator of the base language does that? All, I guess,
but I miss reading that.

Am I right in thinking that using "i;octet", the left side " B " is
equal to "B" (white space is stripped from the left side), but "B"
is not equal to " B " (white space is not stripped from the right side)?

Is white space on the right side an error when used with "eq"?

Michael

---

From: Alexey Melnikov <[email-masked]>
Date: Wed, 14 Sep 2005 17:17:28 +0100

Michael Haardt wrote:

>I miss a list of changes.  Aren't there any?
>
Only explanation what things like "gt", "eq", etc. mean

>I miss a table of contents, too.
>  
>
I believe RFC editor can address this, once the document is approved.

>And which comparator of the base language does that? All, I guess,
"relational" draft is not the place for listing such comparators. There 
is a separate document (currently draft-newman-i18n-comparator-05.txt) 
that defines the comparator registry.

---

From: "Mark E. Mallett" <[email-masked]>
Date: Tue, 20 Sep 2005 11:57:23 -0400

(Sorry to weigh in so late.)

> is not equal to " B " (white space is not stripped from the right side)?

That's reasonable for values that are derived from message elements, I
would want a header field value or an address being tested to be
stripped of its leading spaces.  But that section also defines the left
side of a comparison as always being "from the message" which is hard to
apply to the "string" verb from the variables extension.

Speaking of which, 3.1 says:

 >  Leading and trailing white space MUST be removed from the value of
 >  the message for the comparison.  White space is defined as 
 >
 >      SP / HTAB / CRLF
 >
 >  A value from the message is considered the left side of the relation. 

The first sentence says "the value of the message" where it probably
means to say "the value from the message" or "a value from the message" .

Some other random comments, some significant, some not..

Some of the examples use domain names like "example.com.invalid" --
why not just "example.com" ?  Not that I expect to ever see a ".invalid"
TLD but example.tld is supposed to be for examples.

Also, the examples should use "elsif" rather than "elseif".

Some of the examples refer to a test as "being" true or false, and
others refer to the test as "returning" true or false.  I think
"returning" is a bad word, and would prefer e.g. "evaluates to" .

Should the "extended example" (section 5) require "fileinto" ?

The last "allof" in the extended example is missing a closing paren.

mm

---

From: Cyrus Daboo <[email-masked]>
Date: Tue, 20 Sep 2005 12:23:01 -0400

My 3431bis comments:

--
§2 ¶1

Change:

   MUST be declared a require clause as defined in [SIEVE].

To:

   MUST be declared in a require clause as defined in [SIEVE].

                    ^^

--
§2 and elsewhere: change [ACAP] reference to i18n document.

--

Various places: 'syntax' -> 'usage'

--
§3 perhaps there ought to be an informative reference to 'C', or at least 
define it somewhere as 'the C programming language'.

--
§3.2

The phrase 'number of recipients' is not strictly right as From addresses 
can be tested and that is not a 'recipient'.

Exactly what are we talking about here? The number of 'addr-spec' elements 
as defined by 2822?

Also, I am not sure about the comment on groups. A group can have a list of 
email addresses included - do those get counted or not? It would be useful 
to have an example illustrating this if its not clear. Perhaps text should 
be:

	Group names are ignored, but the list of addresses in a group,
	if provided, are counted.

--
§3.2 ¶6

Change:

	comparing the total number of "to" and "cc" addresses;

To:

	compares the total number of "to" and "cc" addresses;
	
	      ^^

--
Examples: all of the examples use [...] around single string-list items, 
which is not strictly necessary. I always prefer to see the most 'compact' 
syntax representation where ever possible. This is just a personal 
preference.

--
§5 Examples:

Change:

	elseif

To:

	elsif

--

§5: the 'if allof' statement is missing a closing right-paren.

--

§5: 'fileinto' needs to be in the 'require' statement.

--
§8: update SIEVE reference to 3028bis.

-- 
Cyrus Daboo

---

From: Philip Guenther <[email-masked]>
Date: Tue, 20 Sep 2005 15:41:57 -0700

[My apologies for the delay in this as well.  Barry, you may scowl at me
when we next meet.]

The leading broilerplate is out of date.  Possibly the trailing
broilerplate too.

I would suggest striking the second sentence of section 2 ("Any
comparator used..."), as this document is not the source of that
requirement and the list of exempted comparators is no longer accurate.

In the third paragraph of section 3.1, I think "of the message" should
be changed "from the message".

The last comma in section 3.1 ("...considered true[,] if any pair...")
should be removed 

In section 3.2, the phrasing of what the various tests do is a bit odd.
The first paragraph talks about counting entities, but then there's no
further mention of "entities".  If that first paragraph is left as is,
then I think the following paragraphs should be phrased more like:
   When used with the "address" test, the contained "addr-spec" elements
   [RFC2822] are the counted entity.  Note that addr-specs contained in
   group addresses are counted, but the group itself does not affect the
   count.

   When used with the "envelope" test, any envelope-part whose value is
   non-empty will contribute one (1) to the count.  In particular, the
   "to" envelope-part will always contribute exactly one.

(We need to leave the door open for other envelope-parts to be defined.)

   When used with the "header" test, the instances of the named fields
   are the counted entity.  The values of the instances has no effect on
   the count.

...though that text seems clunky too.

The reference to MATCH-TYPE in the ABNF will be rejected as there's no
previous definition of the MATCH-TYPE non-terminal, not even in the
base-spec, so it can't be added to with "=/".  I'm not sure what the
best fix for this is.

Philip Guenther