Re: [PATCH] connected: always use partial clone optimization

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 30, 2020, 18:09 UTC
Message-ID: <xmqqa73x90jw.fsf@gitster.c.googlers.com>
In-Reply-To: <20200330160412.120614-1-jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 18 quoted lines

>> Jonathan Tan <jonathantanmy@google.com> writes:
>> 
>> >> Sounds good.  Jonathan?  I've squashed Josh'es Reviewed-by, but I
>> >> will refrain from merging it to 'next' just yet to see if you too
>> >> like the proposed code structure.
>> >
>> > I think that this is a local enough concern that going either way won't
>> > paint us into a corner, so if what's in
>> > jt/connectivity-check-optim-in-partial-clone is OK, I prefer using that
>> > to reduce churn.
>> 
>> If you do not think their improvement is not much of improvement,
>> then please say so.
>
> Yes, I don't think that their improvement is much of an improvement. If
> we were to split up the logic into functions, one of the functions would
> need to be documented as "Return true if all objects returned by 'fn'
> exist in promisor packs.

So we have a stronger basis to reject the different code structure, and I think it makes sense. Which is a better reason to give than "it is a local enough concern and we can do so later if we wanted to". We probably do not want to anyway, right?

Thanks.  Let's mark the topic as ready for 'next'.