Subject: Re: Patch: Flush and Reopen
From: Martin Owens <doctormo@gmail.com>
To: Austin Clements <amdragon@MIT.EDU>
In-Reply-To: <20110912002327.GC23603@mit.edu>
References: <1315445674.32058.183.camel@delen>
	<CAH-f9WtuYYuksfTHL1ZMmoRF6kyL7iFu9BXDzGn-wWGniSvf9w@mail.gmail.com>
	<1315536861.2435.34.camel@delen> <87obyuj7q1.fsf@zancas.localnet>
	<1315590950.2435.59.camel@delen>
	<CAH-f9WtuL48RtaJ9njxHyCy5cqhhrxdz+30ydE11rq41Lu06pA@mail.gmail.com>
	<1315615433.2435.62.camel@delen>  <20110912002327.GC23603@mit.edu>
Content-Type: text/plain; charset="UTF-8"
Date: Sun, 11 Sep 2011 21:03:23 -0400
Message-ID: <1315789403.2435.74.camel@delen>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Cc: Notmuch developer list <notmuch@notmuchmail.org>
Precedence: list

On Sun, 2011-09-11 at 20:23 -0400, Austin Clements wrote:
> BTW, in the future, you should send patches inline (see the patch
> formatting guide I linked to earlier for easy ways to do this).  It
> makes them much easier to review and reply to.

I tried to do this, my email client doesn't allow this sort of mucking
about. sorry guys bare with the patch file.

> I assume you've tested it?

Absolutely, I have my testing framework and it would fail quite badly if
either flush or reopen failed to work in either py binding or lib.

> Notmuch is quite consistent about indentation, though the rule may not
> be something you'd expect coming from Python.  It uses four space
> indentation, but each group of eight spaces is replaced with a tab, so
> for example

I see, I'm going to forgo the rage and just do it.

> Even if you found the above two changes necessary, they shouldn't be
> included in this patch because they aren't related to reopen/flush.

There were intended to be removed. Should be gone now.

> Both catch blocks should set notmuch->exception_reported = TRUE;
> (notmuch_database_close happens to be the one exception to this since
> it's closing the database; see other catch blocks in lib/database.cc).

Added.

> The catch block in notmuch_database_flush shouldn't appear as changed
> in the diff; you should leave the spacing the way it originally was. 

The spacing is mostly the same now, I removed a word from the error
message deliberately before I noticed it was in the original text. But
it shouldn't be too important.

Regards, Martin Owens