On Tue, Jun 8, 2021 at 4:42 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Elijah Newren <newren@xxxxxxxxx> writes: > > > On Mon, Jun 7, 2021 at 5:26 PM Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote: > >> > >> Thanks everyone for your reviews. I believe I've addressed all review > >> comments, including the one from Elijah about the test failing with > >> sha256 (which turns out to be because I didn't add a call to > >> setup_git_directory(), which the other test helpers do). > > > > Thanks for fixing those up. I spotted some minor nits/questions, but > > nothing big. > > > > Looks like Junio did spot some bigger items...which raises a question > > for me. I have a series ... > > Do you mean, by "bigger items", that we may want to turn it around > to have repo extension data to the in-core repository structure? Yes. > > (https://lore.kernel.org/git/pull.969.git.1622856485.gitgitgadget@xxxxxxxxx/) > > that also touches partial clones. Our series are semantically > > independent, but we both add a repository parameter to > > fetch_objects(). So we both make the same change, but you also make > > additional nearby changes, resulting in two trivial conflicts. ... > > I can sort of see how the above plan would work if we are not going > to fix the "keep only the partialclone related extension thing, > instead of solving the larger structural problem that the current > arrangement ignores that repository extensions are per repository". > But wouldn't that leave us with two series with technical debt? > Also, if Jonathan's series fixes the "bigger item", would the above > "proceed more or less independently or rebase one on top of the > other" plan work well without making the same fix in yours? My series is completely independent of the partialclone extension stuff. My series merely adds the recording of a single statistic (number of fetched objects) to the partial clone stuff; everything else is higher level diffcore-rename and merge-ort stuff. > I guess a better first step would be to stop, think and decide what > to do with the "bigger" thing---if only to dismiss it with a firm > declaration that we would never do such a fix and move extension > data piecemeal to relevant subsystems, so that we'd reduce conflicts > in the future, as I am reasonably sure that the "bigger item" will > be tempting to fix even after the two series lands, and doing so at > that time would be twice larger surgery. I don't understand how you think the partialclone extension stuff is relevant to my series at all. My changes to promisor-remote.c are just a couple lines, and if he expands or rearranges his work, the amount of conflicts can't really get any bigger because there's only a few lines on my side for it to conflict with.
