From: Xing Xin <bupt_xingxin@163.com>
Date: Tue, 08 Oct 2024 03:38:51 GMT
Subject: Re:Re: [PATCH v2 2/5] remote: introduce remote.<name>.serverOption configuration
Message-ID: <e927c65.4aed.1926a354fdb.Coremail.bupt_xingxin@163.com>
In-Reply-To: <ZwOaR5OqdLaS4cu6@pks.im>

At 2024-10-07 16:22:31, "Patrick Steinhardt" <ps@pks.im> wrote:
>On Mon, Sep 23, 2024 at 12:17:55PM +0000, Xing Xin via GitGitGadget wrote:
>> diff --git a/Documentation/config/remote.txt b/Documentation/config/remote.txt
>> index 8efc53e836d..8ba48573fb6 100644
>> --- a/Documentation/config/remote.txt
>> +++ b/Documentation/config/remote.txt
>> @@ -95,3 +95,13 @@ remote.<name>.partialclonefilter::
>>  	Changing or clearing this value will only affect fetches for new commits.
>>  	To fetch associated objects for commits already present in the local object
>>  	database, use the `--refetch` option of linkgit:git-fetch[1].
>> +
>> +remote.<name>.serverOption::
>> +	When no `--server-option=<option>` argument is given from the command
>> +	line, git will use the values from this configuration as a default list of
>
>This description feels a bit inelegant to me. How about the following:
>
>    The default set of server options used when fetching from the
>    remote. These server options are overridden by the
>    `--server-option=` command line arguments.

Thanks for the help! Applied to the new patch (with some minor modifications).

>> @@ -125,6 +126,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
>>  	struct remote *ret;
>>  	struct remotes_hash_key lookup;
>>  	struct hashmap_entry lookup_entry, *e;
>> +	struct string_list server_options = STRING_LIST_INIT_DUP;
>>  
>>  	if (!len)
>>  		len = strlen(name);
>> @@ -143,6 +145,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
>>  	ret->name = xstrndup(name, len);
>>  	refspec_init(&ret->push, REFSPEC_PUSH);
>>  	refspec_init(&ret->fetch, REFSPEC_FETCH);
>> +	ret->server_options = server_options;
>
>I was wondering at first where we actually populate the
>`server_options`, until I realized that this is only done to initialize
>the `ret->server_options`. I think it would be less confusing if this
>used `string_list_init_dup()` instead of the local variable.

Make sense.

Xing Xin