RE: [PATCH 10/10] lower core.maxTreeDepth default to 2048

From: rsbecker@nexbridge.com <rsbecker@nexbridge.com>
Date: Aug 31, 2023, 21:59 UTC
Message-ID: <096e01d9dc56$6d9f12a0$48dd37e0$@nexbridge.com>
In-Reply-To: <xmqq7cpaudke.fsf@gitster.g>

On Thursday, August 31, 2023 5:06 PM, Junio C Hamano wrote:

Show 10 quoted lines

>Jeff King <peff@peff.net> writes:
>
>> On Thu, Aug 31, 2023 at 12:39:37PM +0200, Oswald Buddenhagen wrote:
>>
>>> On Thu, Aug 31, 2023 at 02:23:20AM -0400, Jeff King wrote:
>>> > But I thought that
>>> > following the sequence of logic (from "4096 is probably OK" to
>>> > "whoops, it's not") had some value to share.
>>> >
>>> of course, but you can just integrate that into the squashed commit

message.

Show 7 quoted lines

>>> having it all in one place makes it easier to follow.
>>
>> Yes, though I think having it as a separate patch makes it easier to
>> revisit later (e.g., by reverting or by replacing the patch during a
>> re-roll).
>
>I am on the fence.  Having it squashed into the same step as it was

introduced may

>reduce the patch count, but then it would not be easy to explain why 2048

is a

>reasonable default at that step when no code actually uses the variable, so

the end

>result is not all that easier to follow and read, as that earlier step

would be

>handwaving
>"2048 is good at the end of the series, trust me", unlike having it at the

end.  When

>4096 is introduced as a "random number that seems larger than large enough"

in the

>earlier step, it might be worth mentioning that it is a tentative default

and may turn

>out to be larger than necessary in which case we may want to shrink it ;-)

I have been trying to figure out the implications of this and went down the wrong rabbit hole. Are we taking about the tree depth of the underlying Merkel Tree (no) or the tree-ish thing representing the file system (apparently yes). In this case, a practical depth of 2048 hits the exact max path size on the NonStop platform, so I have no issue there. My concern is one of terminology. My assumption of what maxTreeDepth meant, from other terminology used in git, seemed (wrongly) to align with the use of --depth=n where n<maxTreeDepth parameters for commands like fetch. From a user intuition (arguably, if I have any here) is that the parameter should be more of a path nomenclature, like maxPathHeight or maxHierarchyHeight rather than what is currently in flight. Just my opinion and I'm fine no matter which way.

--Randall -- Brief whoami: NonStop&UNIX developer since approximately UNIX(421664400) NonStop(211288444200000000) -- In real life, I talk too much.