On Thu, Jul 07, 2022 at 11:50:34AM -0700, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > > If so, do you prefer to go straight there in patch 3 (and drop patch 2, > > keeping the unborn setup inline), or do you prefer to see it on top? > > Normally I'd suggest the former, but I worry that doing it all in one > > patch means it's reorganizing the code _and_ changing the behavior all > > at once, which is harder to reason about. And I don't see an easy way to > > reorganize the code without changing the behavior. > > Either way is fine, but the "go straight there" approach may work > better, I suspect. Yeah, the diff turned out less noisy than I'd feared. So here's a v2. It does the refactoring we've been discussing, which is now in patch 2 (since the extra function is no longer needed). And then it was actually pretty easy to fix the other weird "your unborn master does not match the remote's master" problem on top. I _think_ that's the right thing to be doing, but see the discussion in patch 3. I'll skip the range diff, which is mostly unreadable (the only readable part is that I did s/empty/unborn/ in the tests, as discussed). [1/3]: clone: drop extra newline from warning message [2/3]: clone: propagate empty remote HEAD even with other branches [3/3]: clone: use remote branch if it matches default HEAD builtin/clone.c | 58 ++++++++++++++++++++++------------------- t/t5605-clone-local.sh | 16 +++++++++--- t/t5702-protocol-v2.sh | 59 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 29 deletions(-) -Peff
