Skip to main content

Last Call Review of draft-ietf-jmap-sieve-16
review-ietf-jmap-sieve-16-artart-lc-salz-2024-01-29-00

Request Review of draft-ietf-jmap-sieve
Requested revision No specific revision (document currently at 22)
Type Last Call Review
Team ART Area Review Team (artart)
Deadline 2024-02-01
Requested 2024-01-18
Authors Kenneth Murchison
I-D last updated 2024-01-29
Completed reviews Genart Last Call review of -17 by Ines Robles (diff)
Secdir Last Call review of -19 by Mohit Sethi (diff)
Artart Last Call review of -16 by Rich Salz (diff)
Assignment Reviewer Rich Salz
State Completed
Request Last Call review on draft-ietf-jmap-sieve by ART Area Review Team Assigned
Posted at https://mailarchive.ietf.org/arch/msg/art/MtR77hWAeRVusZaF_rChBM2LLg8
Reviewed revision 16 (document currently at 22)
Result Ready w/nits
Completed 2024-01-29
review-ietf-jmap-sieve-16-artart-lc-salz-2024-01-29-00
I am doing the Artart directorate review for the jmap-sieve document. This is
primarily for the appropriate ADs. Authors should consider this like any other
LC comments.

I am not an expert on jmap or sieve, and have moderate familiarity with json,
so caveat lector.

In my view this has nits that can be fixed without controversy as they are all
editorial.

I was surprised that the Abstract did not include more text from Section 1.

Sec 1, is 8620 the "core specification"?  If so, it should be described as such
two paragraphs above when it is first used. An example linebreak could be
useful. I am surprised a positive indication, such as a trailing backslash,
isn't used.

Sec 1.1 and 1.2 could be combined.

Sec 1.3.1, I am completely befuddled by what "The value of this property in an
account's accountCapabilities property is an object that MUST contain the
following information on per-account server capabilities:" property is
mentioned.  The above one, which is a string? Or some unnamed object which
contains the elements specified below it. Maybe it's just cut+paste
duplication. Consider a worked example.

Sec 2, I don't see why the requirements around the script "blob" are given in
the "blobId" field; they should be somewhere else. The "isActive" wording is a
little clunky.  Perhaps "At most one script may be used to filter incoming
messages, indicated by having its isActive field set. Keep the last sentence.

Sec 2 all examples.  Nice to see them.  I cannot comment on their accuracy, I
assume the WG and other experts verified them. I think the parens around the
URL template should be removed. You can say, the examples below assume that the
URL template is "/host/...." etc and I think it is better to use backslash on
the continued line, but good that you are calling it out as the only wrapped
part.  (It is the only wrapper part, right? If so, say so).