Karthik Nayak <karthik.188@xxxxxxxxx> writes:
> /*
> + * Given a refname, return 1 if the refname matches with one of the patterns
> + * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
> + * and so on, else return 0. Supports use of wild characters.
> + */
> +static int match_name_as_path(const char **pattern, const char *refname)
> +{
I wonder why this is not "match_refname", in other words, what the
significance of "as path" part of the name? If you later are going
to introuce match_name_as_something_else() and that name may not be
a refname, then this naming is perfectly sane. If such a function
you will later introduce will still deal with names that are only
refnames, then match_refname_as_path() would be sensible. Otherwise
this name seems overly long (i.e. "as_path" may not be adding value)
while not being desctiptive enough (i.e. is meant to be limited to
refnames but just says "name").
Just being curious.
> @@ -851,40 +888,16 @@ static int grab_single_ref(const char *refname, const unsigned char *sha1, int f
> return 0;
> }
>
> - if (*cb->grab_pattern) {
> -...
> - }
> + if (*cb->grab_pattern && !match_name_as_path(cb->grab_pattern, refname))
> + return 0;
>
> - /*
> - * We do not open the object yet; sort may only need refname
> - * to do its job and the resulting list may yet to be pruned
> - * by maxcount logic.
> - */
> - ref = xcalloc(1, sizeof(*ref));
The comment still is a good reminder for those who will later touch
this grab_single_ref() function to make them think twice.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html