Re: [PATCH v2 3/4] apply: remove the_repository global variable

From: johncai86@gmail.com <johncai86@gmail.com>
Date: Oct 3, 2024, 18:28 UTC
Message-ID: <DCEF5258-BFF7-4667-A538-0BEED2CB70AA@gmail.com>
In-Reply-To: <xmqqwmirzr32.fsf@gitster.g>

Hi Junio,

On 1 Oct 2024, at 13:10, Junio C Hamano wrote:

Show 13 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
>
>> I disagree with this statement. If code isn't prepare to not handle a
>> `NULL` repository we shouldn't fall back to `the_repository`, but we
>> should instead prepare the code to handle this case. This of course
>> requires us to do a ton of refactorings, but that is the idea of this
>> whole exercise to get rid of `the_repository`.
>
> I agree.  To me, the patch was screaming that the author was not
> prepared to go the whole nine yards, though.  Adding back the
> explicit reference to "the_repository" as a fallback is the next
> best thing to do, pushing the "problem" closer to where it is.
>

Indeed, I did not do my due diligence here instead of assuming all layers that look at the repo argument do the right thing.

>> If a command cannot be converted to stop using `the_repository` right
>> now we should skip it and revisit once all prerequisites have been
>> adapted accordingly.

Looks like it’d be preferable if I just drop this patch from the series as it will require a larger refactor.

Thanks John

>
> That is also a viable approach.