Re: [PATCH 1/2] Add notmuch_database_flush method

[Adrien Bustany <adrien@bustany.org>]

Le 17/10/2012 18:53, Ethan Glasser-Camp a écrit :
> Adrien Bustany <adrien@bustany.org> writes:
>
>> This method explicitly flushes the pending modifications to disk. It is
>> useful if your program has various threads, each with a read only DB and
>> one writer thread with a read/write DB. In that case, you most likely
>> want the writer to sync the changes to disk so that the readers can see
>> them, without having to close and reopen the database completely.
>
> These patches are pretty straightforward. But to conform to notmuch style..
>
>> +notmuch_status_t
>> +notmuch_database_flush(notmuch_database_t *notmuch)
>> +{
>> +	notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
>
> Indent is 4 spaces. (You have tabs here, which are 8 spaces, according
> to devel/STYLE.)
>
>> +	try {
>> +	if (notmuch->xapian_db != NULL &&
>
> if should be more indented than try. (So when you pull try back to 4
> spaces, leave if at 8 spaces.)

Sorry about that... I think I copied the style from 
notmuch_database_close, but my editor was set to have 4-space tabs, so I 
got confused. I'll send a fixed version of the patches.

>
>> +	    notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE)
>> +		(static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->flush ();
>
> This line is 90 characters, and will remain at 86 once you indent using
> the in-house style. I'm not sure if it's worth reformatting.
> notmuch_database_close calls flush() using exactly the same 86-character
> line. I'd say "don't make it worse", but personally I think breaking
> this line might be worse.
>

Yes, I also think breaking this line will not make things prettier :-/

> Ethan
>

Thanks for the review!

Adrien