Subject: Re: [PATCH v2 4/9] bundle-uri: serve bundle.* keys from config

From: Victoria Dye <[email-masked]>
Date: Mon, 28 Nov 2022 17:00:56 -0800

Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <[email-masked]>
> 
> Implement the "bundle-uri" protocol v2 capability by populating the
> key=value packet lines from the local Git config. The list of bundles is
> provided from the keys beginning with "bundle.".
> 
> In the future, we may want to filter this list to be more specific to
> the exact known keys that the server intends to share, but for
> flexibility at the moment we will assume that the config values are
> well-formed.

This patch looks good - the implementation is pretty straightforward ("send
a config value if its key matches 'bundle.*'), and the tests cover both
single and multiple bundle URIs returned by the server.

> 
> Signed-off-by: Derrick Stolee <[email-masked]>
> ---
>  bundle-uri.c                          | 16 +++++++++++-
>  t/lib-t5730-protocol-v2-bundle-uri.sh | 35 +++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/t/lib-t5730-protocol-v2-bundle-uri.sh b/t/lib-t5730-protocol-v2-bundle-uri.sh
> index c327544641b..000fcc5e20b 100644
> --- a/t/lib-t5730-protocol-v2-bundle-uri.sh
> +++ b/t/lib-t5730-protocol-v2-bundle-uri.sh
> @@ -158,6 +158,8 @@ test_expect_success "ls-remote with $T5730_PROTOCOL:// using protocol v2" '
>  	[bundle]
>  		version = 1
>  		mode = all
> +	[bundle "only"]
> +		uri = $T5730_BUNDLE_URI_ESCAPED

Ah, okay, this explains why the 'bundle.only.uri' config was added to the
test in the last patch [1]. But, if the config is only being served in this
patch, shouldn't that test change be moved to this patch? 

[1] https://lore.kernel.org/git/c326[email-masked]/

>  	EOF
>  	GIT_TRACE_PACKET="$PWD/log" \
>  	test-tool bundle-uri \