Re: [PATCH] Clear --exclude list after 'git rev-parse --all'

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 24, 2018, 09:24 UTC
Message-ID: <xmqqr2gfn1cy.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CAHc6FU6YLeTBU02k-Md7+H5kJDq2RXZmAXMaq8ZkiuSmZFXbwQ@mail.gmail.com>

Andreas Gruenbacher <agruenba@redhat.com> writes:

Show 12 quoted lines

>> All other glob options do show_reference with for_each_ref_in() and
>> then calls clear_ref_exclusion(), and logically the patch makes
>> sense.
>>
>> What is the "problem" this patch fixes, though?  Is it easy to add a
>> new test to t/6018-rev-list-glob.sh to demonstrate that "--glob" (or
>> whatever that clears exclusion list without this patch) works
>> correctly but "--all" misbehaves without this change?
>
> The test suite doesn't cover clearing the exclusion list for any of
> those rev-parse options and I also didn't write such a test case. I
> ran into this inconsistency during code review.

That is why I asked what "problem" this patch fixes. Without answering that question, it is unclear if the patch is completing missing coverage for "--all", or it is cargo culting an useless clearing done for "--glob" and friends to code for "--all" that did not do the same useless clearing. IOW, there are two ways to address the "inconsistency", and the proposed log message (nor your answer above) does not make a convincing argument why adding the same code to the "--all" side is the right way to achieve consistency---rather than removing the call to clear from the existing ones.

Thanks.