Re: [PATCH 1/2] ref-filter: refactor do_merge_filter()

From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Aaron Lipman <alipman88@gmail.com>, Git List <git@vger.kernel.org>
Subject: Re: [PATCH 1/2] ref-filter: refactor do_merge_filter()
Date: Fri, 18 Sep 2020 10:01:22 -0700	[thread overview]
Message-ID: <xmqqtuvvm331.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <CAPig+cRXJ5hCEM0dFsSSnGWaYu76gPhpM3fV4FaV+Db4r6dX4g@mail.gmail.com> (Eric Sunshine's message of "Fri, 18 Sep 2020 01:03:00 -0400")

Eric Sunshine <sunshine@sunshineco.com> writes:

> One thing that this commit message lacks is a high-level explanation
> of why these changes are being made. One possible rewrite would be:
>
>     ref-filter: make internal reachable-filter API more precise
>
>     The internal reachable-filter API is a bit loose and imprecise; it
>     also bleeds unnecessarily into the public header. Tighten the API
>     by:
>
>     * renaming do_merge_filter() to reach_filter()
>
>     * separating parameters to explicitly identify what data is used
>       by the function instead of passing an entire ref_filter_cbdata
>       struct
>
>     * renaming and moving internal constants from header to source
>       file

Sounds good.  Aaron?

>> +static void reach_filter(struct ref_array *array,
>> +                        struct commit_list *check_reachable,
>> +                        int include_reached)
>>  {
>>         [...]
>> +       if (!check_reachable)
>>                 return;
>
> I would probably drop this conditional return altogether ...

As you said downthread, this part is fine as-is.