From: Karthik Nayak <karthik.188@gmail.com>
Date: Wed, 02 Sep 2015 12:48:35 GMT
Subject: Re: [PATCH v15 06/13] ref-filter: add option to filter out tags, branches and remotes
Message-ID: <CAOLa=ZSRZHWMRh759dGVd4hHebpnnCA60TZoXoY-iqPa=t-e0Q@mail.gmail.com>
In-Reply-To: <xmqqoahlpibt.fsf@gitster.mtv.corp.google.com>

On Wed, Sep 2, 2015 at 9:45 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>>>> +             if (filter->kind == FILTER_REFS_BRANCHES)
>>>> + ret = for_each_fullref_in("refs/heads/", ref_filter_handler,
>>>> &ref_cbdata, broken);
>>>> +             else if (filter->kind == FILTER_REFS_REMOTES)
>>>> + ret = for_each_fullref_in("refs/remotes/", ref_filter_handler,
>>>> &ref_cbdata, broken);
>>>> +             else if (filter->kind == FILTER_REFS_TAGS)
>>>> + ret = for_each_fullref_in("refs/tags/", ref_filter_handler,
>>>> &ref_cbdata, broken);
>>>> +             else if (filter->kind & FILTER_REFS_ALL)
>>>> + ret = for_each_fullref_in("", ref_filter_handler, &ref_cbdata,
>>>> broken);
>>>
>>> This if/else if/else if/ cascade and ...
>>
>> Did you notice the "==" for others and "&" for the ALL?
>
> I didn't.  Thanks for pointing it out.
>
> So the point of the earlier part of the cascade is to optimize for
> common cases?  If that is the case, it probably deserves some
> commenting.  I also suspect that a table-based control might be
> easier to maintain, but that kind of change might fall into the
> category of premature optimization.

Yes, thats the point.
Will add a comment, thanks :)

-- 
Regards,
Karthik Nayak