[PATCH v6 06/14] reindex: drop all properties named with prefix "index."
Daniel Kahn Gillmor
dkg at fifthhorseman.net
Thu Oct 19 22:24:50 PDT 2017
On Thu 2017-10-19 08:52:46 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <dkg at fifthhorseman.net> writes:
>
>> + ret = notmuch_message_remove_all_properties_with_prefix (message, "index.");
>> + if (ret) {
>> + INTERNAL_ERROR ("failed to remove index.* properties");
>> + goto DONE;
>> + }
>
> 1) INTERNAL_ERROR is fatal, so the goto is unneeded / confusing.
>
> 2) Is there no sensible return value here? The function already has an
> error return path. I see there is one INTERNAL_ERROR there so maybe
> similar reasoning applies and a TODO is enough.
I suppose the error returned by
notmuch_message_remove_all_properties_with_prefix() is the value that we
should return. I've switched this to a simple goto DONE in my gitlab
repo.
However, i note that an error here will be (from the outside)
indistinguishable from an error in _notmuch_database_ensure_writable or
_notmuch_message_remove_indexed_terms() immediately above, so i've added
a small TODO anyway.
thanks for the reviews, bremner. please keep them coming! (or tell me
when you're done so i can send v7 to the list with resolutions to all
these nitpicks)
--dkg
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20171020/63685b10/attachment.sig>
More information about the notmuch
mailing list