git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>, git@vger.kernel.org
Subject: Re: [PATCH 10/10] lower core.maxTreeDepth default to 2048
Date: Thu, 31 Aug 2023 14:06:25 -0700	[thread overview]
Message-ID: <xmqq7cpaudke.fsf@gitster.g> (raw)
In-Reply-To: <20230831174215.GA3208283@coredump.intra.peff.net> (Jeff King's message of "Thu, 31 Aug 2023 13:42:15 -0400")

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.
>> 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 ;-)



  reply	other threads:[~2023-08-31 21:06 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-31  6:17 [PATCH 0/10] tree name and depth limits Jeff King
2023-08-31  6:17 ` [PATCH 01/10] tree-walk: reduce stack size for recursive functions Jeff King
2023-08-31  6:19 ` [PATCH 02/10] tree-walk: drop MAX_TRAVERSE_TREES macro Jeff King
2023-08-31  6:19 ` [PATCH 03/10] tree-walk: rename "error" variable Jeff King
2023-08-31  6:20 ` [PATCH 04/10] fsck: detect very large tree pathnames Jeff King
2023-08-31  6:21 ` [PATCH 05/10] add core.maxTreeDepth config Jeff King
2023-08-31  6:21 ` [PATCH 06/10] traverse_trees(): respect max_allowed_tree_depth Jeff King
2023-08-31  6:21 ` [PATCH 07/10] read_tree(): " Jeff King
2023-08-31  6:22 ` [PATCH 08/10] list-objects: " Jeff King
2023-08-31  6:22 ` [PATCH 09/10] tree-diff: " Jeff King
2023-08-31  6:23 ` [PATCH 10/10] lower core.maxTreeDepth default to 2048 Jeff King
2023-08-31 10:39   ` Oswald Buddenhagen
2023-08-31 17:42     ` Jeff King
2023-08-31 21:06       ` Junio C Hamano [this message]
2023-08-31 21:59         ` rsbecker
2023-08-31 22:31           ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq7cpaudke.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=oswald.buddenhagen@gmx.de \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).