Re: [PATCH 2/4] blame: validate and peel the object names on the ignore list
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 12, 2020, 16:54 UTC
- Message-ID
- <xmqqa6wrqt98.fsf@gitster.c.googlers.com>
- In-Reply-To
- <1fa730c4-eaef-2f32-e1b4-716a27ed4646@web.de>
René Scharfe <l.s.r@web.de> writes:
Show 18 quoted lines
>>>> This preexisting feature is curious. It's even documented ('An
>>>> empty file name, "", will clear the list of revs from previously
>>>> processed files.') and covered by t8013.6. Why would we need
>>>> such magic in addition to the standard negation
>>>> (--no-ignore-revs-file) for clearing the list? The latter
>>>> counters blame.ignoreRevsFile as well. *puzzled*
>>>
>>> I shared the puzzlement when I saw it, but ditto.
>>
>> I don't recall exactly. Someone on the list might have wanted to
>> both counter the blame.ignoreRevsFile and specify another file. Or
>> maybe they just wanted to counter the ignoreRevsFile, and I didn't
>> know that --no- would already do that. I'm certainly not wed to it.
>
> The first step would be to show a deprecation warning, wait a few
> releases and then remove that feature. Not sure the effort and
> potential user irritation is worth the saved conditional, doc lines
> and test. (We already established that I'm lazy.)I do not particularly see the need to. Perhaps when somebody complains the next time?
Show 7 quoted lines
> Anyway, here's the patch: > --- > blame.c | 2 +- > blame.h | 5 +++-- > builtin/blame.c | 16 ++++++++++++---- > object.h | 3 ++- > 4 files changed, 18 insertions(+), 8 deletions(-)
Looks OK to me from a quick scan.