On Tue, Dec 03 2013, Jani Nikula <jani@nikula.org> wrote: > On Tue, 03 Dec 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote: >> On Tue, Dec 03 2013, David Bremner <david@tethera.net> wrote: >> >>> The first patch looks ok. I like the new API overall. >>> >>> 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. >>> >>> Jani Nikula <jani@nikula.org> writes: >>> >>>> +/* >>>> + * XXX: error handling should clean up *all* state created! >>>> + */ >>> is this a warning to future hackers or some current problem? >>> >>>> 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? >>> >>> >>>> + 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? >> >> if that, then also database_path to options ? > > See my reply to David. I agree with your explanation there. > >> I also like this suggestion best of all seen so far, but what if: >> >> #define NOTMUCH_MAJOR_VERSION 0 >> #define NOTMUCH_MINOR_VERSION 17 >> -#define NOTMUCH_MICRO_VERSION 0 >> +#define NOTMUCH_MICRO_VERSION 90 >> >> until changed API/ABI is set into stone (i.e. 0.18.0 at freeze time). > > That would require the user to conditional build with: > > #if NOTMUCH_CHECK_VERSION(0, 17, 90) > /* building against post-0.17 git master or released 0.18 */ > #else > /* building against 0.17 */ > #endif > > instead of the IMHO more natural and accurate: > > #if NOTMUCH_CHECK_VERSION(0, 18, 0) > /* building against post-0.17 git master or released 0.18 */ > #else > /* building against 0.17 */ > #endif True -- I always forget that NOTMUCH_CHECK_VERSION() checks for the value and *newer*. Although there is slight difference I don't think it warrants the use of intermediate values -- the changing API is much bigger issue than this and that is what we have to concentrate on. > > > BR, > Jani. Tomi