From: Derrick Stolee <stolee@gmail.com>
Date: Tue, 20 Apr 2021 12:39:33 GMT
Subject: Re: [PATCH] builtin/repack.c: set a default factor for '--geometric'
Message-ID: <60af51b7-b2a3-8acf-f7a0-49a137daf2c6@gmail.com>
In-Reply-To: <1ecab817396fae6a1cbafde1ca8b3ebfd9ae4c11.1618883241.git.me@ttaylorr.com>

On 4/19/2021 9:47 PM, Taylor Blau wrote:
> The '--geometric=<n>' argument specifies that each pack must contain at
> least 'n' times as many objects as the size of the next-largest pack.
> The factor 'n' is customizable, but setting it to '2' is a sane default.
> 
> Instead of making the factor a required argument, make the argument
> optional with a default value of '2'.

This flexibility is nice.
 
> To ensure that the option is setup correctly, modify the most complex
> test of t7703 to drop the explicit factor.

Good testing.

> --g=<factor>::
> ---geometric=<factor>::
> +-g=[<factor>]::
> +--geometric[=<factor>]::
>  	Arrange resulting pack structure so that each successive pack
> -	contains at least `<factor>` times the number of objects as the
> -	next-largest pack.
> +	contains at least `<factor>` (`2` if unspecified) times the
> +	number of objects as the next-largest pack.

The parenthetical interrupts what `<factor>` means. Perhaps
rearrange:

	Arrange resulting pack structure so that each successive pack
	contains at least `<factor>` times the number of objects as the
	next-largest pack. If `<factor>` is not specified, then `2` is
	used by default.

Rest of the diff looks good.

Thanks,
-Stolee