Last Call Review of draft-ietf-extra-sieve-mailboxid-05
review-ietf-extra-sieve-mailboxid-05-genart-lc-resnick-2020-11-30-00

Request Review of draft-ietf-extra-sieve-mailboxid
Requested rev. no specific revision (document currently at 09)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2020-12-02
Requested 2020-11-18
Authors Bron Gondwana
Draft last updated 2020-11-30
Completed reviews Secdir Last Call review of -06 by Samuel Weiler (diff)
Genart Last Call review of -05 by Pete Resnick (diff)
Assignment Reviewer Pete Resnick 
State Completed Snapshot
Review review-ietf-extra-sieve-mailboxid-05-genart-lc-resnick-2020-11-30
Posted at https://mailarchive.ietf.org/arch/msg/gen-art/lPP_DlIyEbiQOC9YIVVnigkXYzU
Reviewed rev. 05 (document currently at 09)
Review result Ready with Issues
Review completed: 2020-11-30

Review
review-ietf-extra-sieve-mailboxid-05-genart-lc-resnick-2020-11-30

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-extra-sieve-mailboxid-05
Reviewer: Pete Resnick
Review Date: 2020-11-30
IETF LC End Date: 2020-12-02
IESG Telechat date: Not scheduled for a telechat

Summary: Looking good. Just one minor issue and one nit.

Major issues:

None.

Minor issues:

Section 4 says:

   If there is no such mailbox, the "fileinto" action proceeds as it
   would without the ":mailboxid" argument.

But the in the example in section 6, it shows:

       if mailboxidexists "F6352ae03-b7f5-463c-896f-d8b48ee3" {
           fileinto :mailboxid "F6352ae03-b7f5-463c-896f-d8b48ee3"
                               "INBOX.harassment";
       } else {
           fileinto "INBOX.harassment";
       }

That appears correct, but as far as I can tell, it is semantically identical to:

           fileinto :mailboxid "F6352ae03-b7f5-463c-896f-d8b48ee3"
                               "INBOX.harassment";

That is, the rule in section 4 means that fileinto already does an implicit existence check and only uses the named mailbox if the one specified by the mailboxid doesn't exist. It's not that the example is particularly a problem, but it did confuse me for a few minutes while I tried to figure out what it was trying to do. Perhaps if the example was:

       if mailboxidexists "F6352ae03-b7f5-463c-896f-d8b48ee3" {
           fileinto :mailboxid "F6352ae03-b7f5-463c-896f-d8b48ee3"
                               "this.name.will.never.be.used";
       } else {
           fileinto "INBOX.harassment";
       }

or an example that did something other than "fileinto" it would have made a bit more sense. Certainly not absolutely necessary to fix, but a change might improve understanding.

Nits/editorial comments:

In sections 4.1 and 4.2, you have references that appear as "[!@RFC5490]" and "[!@RFC5879]". I assume that's some sort of markdown or other formatting tool mistake.