Elijah Newren [off-list ref] writes:
On Mon, Jun 7, 2021 at 5:26 PM Jonathan Tan [off-list ref] wrote:
quoted
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?
(https://lore.kernel.org/git/pull.969.git.1622856485.gitgitgadget@gmail.com/)
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?
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.
On Tue, Jun 8, 2021 at 4:42 PM Junio C Hamano [off-list ref] wrote:
Elijah Newren [off-list ref] writes:
quoted
On Mon, Jun 7, 2021 at 5:26 PM Jonathan Tan [off-list ref] wrote:
quoted
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.
quoted
(https://lore.kernel.org/git/pull.969.git.1622856485.gitgitgadget@gmail.com/)
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.