[PATCH 2/2] lib: introduce notmuch_database_new for initializing a database handle

[Jani Nikula]

On Tue, 03 Dec 2013, David Bremner <david at tethera.net> wrote:
> The first patch looks ok. I like the new API overall.

Happy to hear that.

> As far as breaking contrib/notmuch-deliver, I'd rather fix
> notmuch-insert than put effort into notmuch-deliver at this point. I
> guess it could be a rough transition for people running notmuch-deliver
> from git.

Agreed. We could fix notmuch insert with patch 1/2 alone.

> Jani Nikula <jani at nikula.org> writes:
>
>> +/*
>> + * XXX: error handling should clean up *all* state created!
>> + */
> is this a warning to future hackers or some current problem?

It's a note to self and reviewers that we need to be sure this
happens... I'm just saying I'm not 100% sure we're doing all of that
now.

>>  notmuch_status_t
>> -notmuch_database_open (const char *path,
>> -		       notmuch_database_mode_t mode,
>> -		       notmuch_database_t **database)
>> +notmuch_database_open (notmuch_database_t *notmuch, const char *path,
>> +		       notmuch_database_mode_t mode)
>>  
>> +/* Initialize a new, empty database handle.
>> + *
>
> I wondered about making the new documentation adhere to doxygen?

I was thinking of fixing either series depending on them getting
merged. I wasn't sure we'd reached consensus on doxygen yet.

>> +    if (notmuch_database_open (notmuch,
>> +			       notmuch_config_get_database_path (config),
>> +			       NOTMUCH_DATABASE_MODE_READ_ONLY))
>
> Would it make any sense to migrate the mode argument to an option
> setting while we're messing with the API?  

My gut feeling says no. Same for the path argument suggested by Tomi.

What would it mean to change the mode while the database is open? Or the
path? I think we'd have to check for this, and fail. Mode is only
meaningful for open, and path for open, create and compact. I think
adding such state modifiers would make the interface feel more complex,
but I'm open to arguments to the contrary.


BR,
Jani.