Re: [PATCH v4 09/10] serve.[ch]: remove "serve_options", split up --advertise-refs code

From: Derrick Stolee <stolee@gmail.com>
Date: Aug 24, 2021, 16:52 UTC
Message-ID: <17efcff7-9a62-046b-ecf9-2cbed84280e9@gmail.com>
In-Reply-To: <patch-v4-09.10-c9a35868933-20210805T011823Z-avarab@gmail.com>

On 8/4/2021 9:25 PM, Ævar Arnfjörð Bjarmason wrote: ...

Show 10 quoted lines

> -struct upload_pack_options {
> -	int stateless_rpc;
> -	int advertise_refs;
> -	unsigned int timeout;
> -	int daemon_mode;
> -};
> -
> -void upload_pack(struct upload_pack_options *options);
> +void upload_pack(const int advertise_refs, const int stateless_rpc,
> +		 const int timeout);

Normally, I would err on keeping a struct that presents the different parameters of a complicated method, since it is easier to add options without modifying all callers. However, in this case we already had some confusion because the 'daemon_mode' option is dependent on the other values.

The fact that these parameters are translated into a 'struct upload_pack_data' immediately within upload_pack() shows that these can be grouped into a "context" object for internal use without needing this pattern for the method signature.

Works for me.

Thanks, -Stolee