Re: [notmuch] [PATCH] Store documents for message-ids we haven't seen

Subject: Re: [notmuch] [PATCH] Store documents for message-ids we haven't seen

Date: Mon, 21 Dec 2009 10:43:16 -0800

To: James Westby, notmuch@notmuchmail.org

Cc:

From: Carl Worth


On Sun, 20 Dec 2009 20:27:32 +0000, James Westby <jw+debian@jameswestby.net> wrote:
>   One case that isn't handled is when we don't find the thread
>   id of the parent, but then find the message itself. I believe
>   this case shouldn't happen, but you never know.

It really shouldn't happen since we are holding a write lock on the
database, (so there's no possible race condition here with another
client delivering the parent message).

But since you almost can't help but detect the case, (just noticing a
NOTMUCH_STATUS_SUCCESS value from _create_for_message_id), please put an
INTERNAL_ERROR there rather than marching along with an incorrect thread
ID.

> +	    // We have yet to see the referenced message, generate a thread id
> +	    // for it if needed and store a dummy message for the parent. If we
> +	    // find the mail later we will replace the dummy.

Call me old-fashioned if you will, but I'd much rather have C style
multi-line comments (/* ... */) rather than these C++-style comments
with //.

> +		    if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
> +			// expected

And I think this comment deserves a complete sentence before the
condition. Something like:

/* We expect this call to create a new document (return NO_DOCUMENT_FOUND) */

-Carl
part-000.sig (application/pgp-signature)

Thread: