Re: [PATCH v2 5/9] bundle-uri client: add boolean transfer.bundleURI setting
- From
Derrick Stolee <derrickstolee@github.com>
- Date
- Dec 2, 2022, 15:38 UTC
- Message-ID
- <7f94bda0-9f08-f179-3e7e-a791a607657e@github.com>
- In-Reply-To
- <4ec06afa-d804-16b1-f140-dfd9ebd912e5@github.com>
On 11/28/2022 8:03 PM, Victoria Dye wrote:
Show 28 quoted lines
> Ævar Arnfjörð Bjarmason via GitGitGadget wrote:
>> From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?=
>> <avarab@gmail.com>
>>
>> The yet-to-be introduced client support for bundle-uri will always
>> fall back on a full clone, but we'd still like to be able to ignore a
>> server's bundle-uri advertisement entirely.
>>
>> The new transfer.bundleURI config option defaults to 'false', but a user
>> can set it to 'true' to enable checking for bundle URIs from the origin
>> Git server using protocol v2.
>
> Thanks for adding this, an "opt-in" approach seems reasonable for
> introducing this feature.
>
>>
>> To enable this setting by default in the correct tests, add a
>> GIT_TEST_BUNDLE_URI environment variable.
>
> This makes sense. I'm less concerned with this environment variable than
> those in patch 2 [1], since it's in line with the test variables that
> enable/disable whole features ('GIT_TEST_SPLIT_INDEX',
> 'GIT_TEST_COMMIT_GRAPH', etc.).
>
> The only thing feedback I can think of would be that this patch could be
> moved to earlier in the series (that is, immediately after creating
> 'transport_get_remote_bundle_uri()'). That said, I don't feel strongly
> either way.It was simple enough to reorder them, so I've done that.
Thanks, -Stolee