On Fri, Jul 22 2022, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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@xxxxxxxxx https://lore.kernel.org/git/cover-v4-00.17-00000000000-20220728T162442Z-avarab@xxxxxxxxx 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!
