Michal Sojka <sojkam1@fel.cvut.cz> writes:

>  
> +/* Returns TRUE iff name and addr is duplicate. */

If you're revising this patch, it would be good to mention the side
effect of this function.

> -process_address_list (const search_context_t *ctx, InternetAddressList *list)
> +process_address_list (const search_context_t *ctx,
> +		      InternetAddressList *list)

It probably doesn't make any difference, but this looks like a needless
whitespace change.

This function definitely needs some comment / pointer to
documention. And probably not to have _my in the name.

> +static void
> +_my_talloc_free_for_g_hash (void *ptr)
> +{
> +    talloc_free (ptr);
> +}
> +

I don't understand the name of the next subtest

> +test_begin_subtest "No --output"
> +notmuch address --output=sender --output=recipients '*' >OUTPUT
> +# Use EXPECTED from previous subtest
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +
> +test_done

nitpick, extra blank lines

So, AIUI, this is all of the series proposed for 0.19. It looks close to
OK to me, modulo some minor style nits. One anonymous commentator on
IRC mentioned the use of module scope variables, I guess in patch
6/10. I'm not sure of a better solution, but it's true in a perfect
world we wouldn't have module local state.

d