Re: [Last-Call] Genart last call review of draft-ietf-extra-sieve-mailboxid-05

[Bron Gondwana <[e-mail]>]

Thanks Pete, fair enough - I can see how a semantically pointless example is a problem.  I'll update that in my copy and wait for any other review changes before uploading again.

And yeah - if the latest mmark is causing artifacts I'll see what I can do about those.  I admit I didn't read through the whole thing in detail again after making the last updates.

Cheers,

Bron

On Tue, Dec 1, 2020, at 17:05, Pete Resnick via Datatracker wrote:
> Reviewer: Pete Resnick
> Review result: Ready with Issues
> 
> 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.
> 
> 
> 

--
  Bron Gondwana, CEO, Fastmail Pty Ltd
  [e-mail]