From: Ævar Arnfjörð Bjarmason Date: Thu, 28 Jul 2022 16:30:28 GMT Subject: Re: [PATCH v3 02/26] submodule--helper: stop conflating "sb" in clone_submodule() Message-ID: <220728.86wnbxjixa.gmgdl@evledraar.gmail.com> In-Reply-To: On Fri, Jul 22 2022, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason 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!