Quoth David Bremner on Aug 23 at 8:58 pm:
> Austin Clements <amdra...@mit.edu> writes:
>
> >>> @@ -1048,7 +1164,8 @@ notmuch_database_get_version (notmuch_database_t
> >>> *notmuch)
> >>> notmuch_bool_t
> >>> notmuch_database_needs_upgrade (notmuch_database_t *notmuch)
> >>> {
> >>> - return notmuch->needs_upgrade;
> >>> + return notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE &&
> >>> + (NOTMUCH_FEATURES_CURRENT & ~notmuch->features);
> >>> }
> >>
> >> Maybe I'm not thinking hard enough here, but how does this deal with a
> >> feature that is needed to open a database in read only mode? Maybe it
> >> needs a comment for people not as clever as Austin ;).
> >
> > I'm not quite sure what you mean. notmuch_database_needs_upgrade
> > returns false for read-only databases because you can't upgrade a
> > read-only database. This was true before this patch, too, though it was
> > less obvious. (Maybe that's not what you're asking?)
>
> Yes, that's what I was asking. I guess it's orthogonal to your patch
> series, but the logic of returning FALSE for read only databases is not
> very intuitive to me (in the sense that "needs upgrade" is not the
> opposite of "can't be upgraded". Your new comment later in the series
> is better, but it would IMHO be even better if you mentioned the read
> only case.
That's a good point. Ideally this should be
"notmuch_database_can_upgrade" or something, which I think would
capture exactly what it means after this series. However, I don't
think it's worth breaking API compatibility given that this is already
how callers use this function (even though that violates the current
library spec).
So, how's this for a more updated doc comment on needs_upgrade?
/**
* Can the database be upgraded to a newer database version?
*
* If this function returns TRUE, then the caller may call
* notmuch_database_upgrade to upgrade the database. If the caller
* does not upgrade an out-of-date database, then some functions may
* fail with NOTMUCH_STATUS_UPGRADE_REQUIRED. This always returns
* FALSE for a read-only database because there's no way to upgrade a
* read-only database.
*/
_______________________________________________
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
