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).