From: Chandra Pratap Date: Sun, 21 Jun 2026 05:37:47 GMT Subject: Re: [PATCH GSoC RFC v13 05/12] fetch-pack: move function to connect.c Message-ID: In-Reply-To: <20260619-ps-eric-work-rebase-v13-5-3d4c7315d2f8@gmail.com> On Fri, 19 Jun 2026 at 20:26, Pablo Sabater wrote: > > write_fetch_command_and_capabilities will be refactored in a subsequent Nit: the rest of this patch's body referes to this function as: `write_fetch_command_and_capabilities()` Let's use that here as well. > commit where it will become a more general-purpose function, making it > more accessible to additional commands in the future. > > To move `write_fetch_command_and_capabilities()` to `connect.c`, we need > to adjust how `advertise_sid` is managed. Previously in `fetch_pack.c`, > `advertise_sid` was a static variable, modified using > `repo_config_get_bool()`. > > In `connect.c`, we now initialize `advertise_sid` at the begining by > directly using `repo_config_get_bool()`. This change is safe because: > > In the original `fetch-pack.c` code, there are only two places that write > `advertise_sid`: > > 1. In function `do_fetch_pack()`: > if (!sever_supports("session_id")) s/sever/server > advertise_sid = 0; > 2. In function `fetch_pack_config()`: > repo_config_get_bool("transfer.advertisesid", &advertise_sid); > > About 1, since `do_fetch_pack()` is only relevant for protocol v1, this > assignment can be ignored, as `write_fetch_command_and_capabilities()` > is only used in v2. > > About 2, `repo_config_get_bool()` is from `config.h` and it's an out-of-box > dependency of `connect.c`, so we can reuse it directly. > > Move `write_fetch_command_and_capabilities()` to `connect.c` Nit: this is a better patch header than "move function to connect.c", since it better describes the exact change we intend to make. Let's use it instead.