From: Derrick Stolee Date: Tue, 12 May 2026 14:33:50 GMT Subject: Re: [PATCH v3 3/7] transport: rename negotiation_tips Message-ID: <16a7342e-015a-4e68-ac1e-45e59313299f@gmail.com> In-Reply-To: On 5/12/26 7:30 AM, Matthew John Cheetham wrote: > On 2026-04-22 16:25, Derrick Stolee via GitGitGadget wrote: >> From: Derrick Stolee >> >> The previous change added the --negotiation-restrict synonym for the >> --negotiation-tips option for 'git fetch'. In anticipation of adding a >> new option that behaves similarly but with distinct changes to its >> behavior, rename the internal representation of this data from >> 'negotiation_tips' to 'negotiation_restrict_tips'. > > Nitpick: s/tips/tip/ .. no trailing s for either the option name, nor > the (old) variable name. The function names do use the plural however. Thanks for the close eye! >> diff --git a/transport-helper.c b/transport-helper.c >> index 4d95d84f9e..0e5b3b7202 100644 >> --- a/transport-helper.c >> +++ b/transport-helper.c >> @@ -754,7 +754,7 @@ static int fetch_refs(struct transport *transport, >>           set_helper_option(transport, "filter", spec); >>       } >> -    if (data->transport_options.negotiation_tips) >> +    if (data->transport_options.negotiation_restrict_tips) >>           warning("Ignoring --negotiation-tip because the protocol does not >> support it."); >>       if (data->fetch) > > Oh! Looks like a place was missed when renaming the preferred option name in > strings. It probably makes sense to do this rename in this patch > (rather than in patch 1) since we're already updating the struct field > name here anyway, but up to you. > > Also do we also want to make it translatable like the others? I will update this as part of the previous patch that handles all the strings, including making it translatable and dropping the capital "I" at the start. Good find. > Aside: I just noticed another '--negotiation-tip' instance in the > `get_commons_through_negotiation` function in send-pack.c. It still uses > the 'tip' option name when forming the shell cmdline. Thanks! I've done a more careful search and confirmed that you found everything I had missed. -Stolee