Re: [PATCH] partial-clone: add a partial-clone test case

From: Junio C Hamano <gitster@pobox.com>
To: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
Cc: git@vger.kernel.org, Derrick Stolee <derrickstolee@github.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH] partial-clone: add a partial-clone test case
Date: Mon, 14 Mar 2022 21:35:57 +0000	[thread overview]
Message-ID: <xmqqwngwnrnm.fsf@gitster.g> (raw)
In-Reply-To: <20220314154638.59046-1-chakrabortyabhradeep79@gmail.com> (Abhradeep Chakraborty's message of "Mon, 14 Mar 2022 21:16:38 +0530")

Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> wrote:
>
>> Is this "doesn't" (documenting current behaviour, without saying if
>> it is wrong or is desired) or "shouldn't" (documenting the desired
>> behaviour, which the current implementation may or may not satisfy)?
>
> The current behaviour is okay and this commit adds the test case for it.
> So, in that sense, I think "shouldn't" is better word.
>
>> That's mouthful.
>
> Sorry if the test name is very long. But, I couldn't think of shorter
> test name than this - to explain what the test case is.

"exact rename does not need to fetch the blob lazily"

>> Lose SP after '>'.
>>
>> 		git -C partial.git log --follow -- new-file.txt >"$(pwd)/trace.txt" &&
>
> Okay.
>
>> Looking at the implementation of the helper, it seems to be prepared
>> to handle negation itself.  Shouldn't this be
>>
>>	test_subcommand_inexact ! fetch <trace.txt
>>
>> instead?
>
> Oops, completely missed it. Correcting it :)
>
>> Why can't you specify what should NOT come before "fetch" in your
>> use of this helper?
>
> Below is the event triggered for non-exact OID rename -
>
> 	git -c fetch.negotiationAlgorithm=noop fetch origin --no-tags --no-write-fetch-head --recurse-submodules=no --filter=blob:none --stdin
>
> Derrick told me to not depend on other flags like
> `-c fetch.negotiationAlgorithm` etc. as they might be changed or omitted
> and as it makes sense to me also. That's why I didn't specify those things.
>
>> I wonder if it was more like this that the original wanted to grep for:
>>
>>	grep '"event":"child_start".*\["git","pack-objects",.*\]'
>
> I don't know about other cases, but in my case, atleast I really wanted
> it.
>
> So, In this scenerio, should I stick with `test_subcommand_inexact` or I
> have to see other helper functions (or make my own) for it?

If you are doing just a single grep, I am not sure why grepping for
"fetch" alone without any helper is insufficient.  In any case,
butchering the "inexcat" helper to loosen it for other existing
users of the same helper does not sound like a good direction to go.

Thanks.