Subject: Re: [PATCH 2/4] ref-filter: add ref-filter API

From: Junio C Hamano <[email-masked]>
Date: Sat, 23 May 2015 10:18:04 -0700

Matthieu Moy <[email-masked]> writes:

> But I think this could be clearer in the code (and/or comment + commit
> message). Perhaps stg like:
>
> struct ref_filter_data /* Probably not the best name */ {
>         struct ref_list list;
>         struct ref_filter filter;
> };
>
> struct ref_list {
>  	int count, alloc;
>  	struct ref_filter_item **items;
> };

If you plan to use ALLOC_GROW() API, name the bookkeeping variables
"alloc" & "nr", unless you have a compelling reason to deviate from
the prevailing practice.

> struct ref_filter {
> 	const char **name_patterns;
> 	/* There will be more here later */
> };

Very good suggestion.

Whatever the final name would be, it is a good idea to separate the
"list of things that are operated on" and the "set of operations to
be applied".  That makes things conceptually cleaner; you can have
multiple of the former operated on with a singleton of the latter
and then their results merged, etc. etc.

And I do not think an array of things that are operated on should
not be named "ref_filter_item".

Surely, the latter "set of operations to be applied" may currently
be only filtering, but who says it has to stay that way?  "I have a
set of refs that represent my local branches I am interested
in. Please map them to their corresponding @{upstream}" is a
reasonable request once you have an infrastructure to represent "set
of refs to be worked on" and "set of operations to apply", and at
that point, the items are no longer filter-items (map-items?).