From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mon, 04 Oct 2021 10:09:24 GMT
Subject: Re: [PATCH 08/11] reset_head(): take struct rebase_head_opts
Message-ID: <e423b101-77e2-02df-ee36-03c692c44e6c@gmail.com>
In-Reply-To: <xmqq8rzc5u2j.fsf@gitster.g>

Hi Junio

On 01/10/2021 22:11, Junio C Hamano wrote:
> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> This function already takes a confusingly large number of parameters
>> some of which are optional or not always required. The following
>> commits will add a couple more parameters so change it to take a
>> struct of options first.
> 
> Would all the members in the struct always be used by the function?
> If not, such a change will *not* solve the "confusingly ... required"
> problem at all.

I'll rephrase to emphasize that it solves the problem of remembering 
which order to pass things in and makes it clear what is being passed

> I am not necessarily against a change to consolidate a bag of
> parameters into a pointer to a struct.  I am against a change that
> is justified with a benefit that the change does not bring to us.

I found the existing api hard to use because it is easy to lose track of 
which position each parameter should be in. With the struct the order 
does not matter and having to set opts.parameter = value makes it clear 
which parameters are being passed.

Best Wishes

Phillip

> Thanks.
>