Re: [WIP/PATCH v4 1/8] for-each-ref: extract helper functions out of grab_single_ref()

From: Karthik Nayak <karthik.188@gmail.com>
Date: May 31, 2015, 17:48 UTC
Message-ID: <556B4954.6040903@gmail.com>
In-Reply-To: <xmqqvbf8abwi.fsf@gitster.dls.corp.google.com>

On 05/31/2015 11:04 PM, Junio C Hamano wrote:

Show 22 quoted lines

> Karthik Nayak <karthik.188@gmail.com> 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.
>

Good Question, This is because in the future I'll eventually add pattern matching for 'tag -l' and 'branch -l' which wont match as path, but would be a regular string match (with wild character support). Hence keeping that in mind I included 'as_path()' in the function name.

Show 6 quoted lines

>
> The comment still is a good reminder for those who will later touch
> this grab_single_ref() function to make them think twice.
>
> Thanks.
>

Yes! It was removed it by mistake.

-- 
Regards,
Karthik