[notmuch] [PATCH 2/4] notmuch: Config option to specify tags to be applied by 'notmuch new'.
Jan Janak
jan at ryngle.com
Wed Nov 25 13:50:08 PST 2009
Hi Bart,
On 25-11 15:56, Bart Trojanowski wrote:
> Jan,
>
> I really want this feature to get in, so I am going to do my best to
> review your code :)
>
> Here are some more sticking points...
>
> > +char **
> > +notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length);
>
> If you are not giving over control of the pointer to the caller please
> return const char * const *.
I followed Carl's style there, in particular the following function:
notmuch_config_get_user_other_email
I can, of course, change that. But maybe we should wait for Carl to see
which way he prefers.
> Similarly...
>
> > + char **new_tags;
>
> ... this should probably be const char **.
That's the same story. I followed user_other_email there.
> Next...
>
> > +char **
> > +notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length)
>
> ... but ...
>
> > + unsigned int count, i;
> > +
> > + if ((tags = notmuch_config_get_new_tags (config, &count)) == NULL)
> > + return;
>
> size_t != unsigned int on all platforms. Please stick with one or the
> other. Note there are a few calls to fix here.
That's a good catch. I will fix that one. Thanks a lot for the review, I
really appreciate that!
-- Jan
More information about the notmuch
mailing list