Re: [PATCH v3 02/26] submodule--helper: stop conflating "sb" in clone_submodule()

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jul 28, 2022, 16:30 UTC
Message-ID: <220728.86wnbxjixa.gmgdl@evledraar.gmail.com>
In-Reply-To: <xmqq8rolc7cu.fsf@gitster.g>

On Fri, Jul 22 2022, Junio C Hamano wrote:

Show 16 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> This looks like a roundabout way to say xstrfmt().
>>
>> Yes, I can fix this and others while I'm at it, but a lot of things like
>> that in this code are funny uses of APIs that we could improve.
>>
>> I think it's probably best to just leave these for now.
>
> Agreed.  We could instead have a separate series to fix API usage
> before these and then build leak-plugging on top, or the other way
> around, and in general "clean then plug" would make it easier to
> review the plugging patches (simply because it would be working on
> clean code, not code that misuses the API in strange ways), but it
> is too late now.  Lets make sure we do not forget to revisit the API
> misuse but lets avoid mixing it into the series.

I ended up doing the opposite of what you suggested here, but not lightly.

When re-rolling the v4 of the leak series I noticed that some of what was suggested I'd fix (and thanks a lot to you and Glen for the reviews) was code that was either dead, or should really belong in a "test-tool" at this point.

So, I could have addressed those by padding the "leak" series with more digressions, but I felt that just cleanly splitting it into a "prep" series and "leak fixes" was better, those two are the just-submitted:

	https://lore.kernel.org/cover-00.20-00000000000-20220728T161116Z-avarab@gmail.com
	https://lore.kernel.org/git/cover-v4-00.17-00000000000-20220728T162442Z-avarab@gmail.com

Sorry if that causes any disruption for you. I noticed that you merged the v3 into your "jch", but it didn't look anywhere close to "next", so getting it more right to begin with seemed like a better trade-off.

Thanks!