Re: Patch: Flush and Reopen

Subject: Re: Patch: Flush and Reopen

Date: Sun, 11 Sep 2011 21:03:23 -0400

To: Austin Clements

Cc: Notmuch developer list

From: Martin Owens


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


Thread: