From: Pablo Sabater Date: Wed, 24 Jun 2026 12:24:37 GMT Subject: Re: [PATCH GSoC RFC v13 05/12] fetch-pack: move function to connect.c Message-ID: In-Reply-To: El dom, 21 jun 2026 a las 7:38, Chandra Pratap () escribió: > > 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. I'll do that, thanks. > > > 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 True, thanks. > > > 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. Okay, I'll use it. Thanks for the review, Pablo