From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 25 Apr 2025 04:34:09 GMT
Subject: Re: [PATCH 1/3] test-tool: add pack-deltas helper
Message-ID: <aAsQwSfr-YvS2Mvh@pks.im>
In-Reply-To: <5d4beb202d6ed842de72928462a10a4f5faa2718.1745430004.git.gitgitgadget@gmail.com>

On Wed, Apr 23, 2025 at 05:40:02PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/t/helper/test-pack-deltas.c b/t/helper/test-pack-deltas.c
> new file mode 100644
> index 00000000000..db7d1c3cd1f
> --- /dev/null
> +++ b/t/helper/test-pack-deltas.c
> @@ -0,0 +1,140 @@
[snip]
> +int cmd__pack_deltas(int argc, const char **argv)
> +{
> +	int N;
> +	struct hashfile *f;
> +	struct strbuf line = STRBUF_INIT;
> +
> +	if (argc != 2) {
> +		usage(usage_str);
> +		return -1;
> +	}
> +
> +	N = atoi(argv[1]);

Is there a reason why we don't use `parse_options()` here? It might make
this tool easier to use and extend going forward, and we wouldn't have
to care about invalid arguments. Right now, we silently accept a
non-integer argument and do the wrong thing.

Patrick