Re: [PATCH v5 0/2] branch: inherit tracking configs

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Dec 10, 2021, 22:48 UTC
Message-ID: <nycvar.QRO.7.76.6.2112102344370.90@tvgsbejvaqbjf.bet>
In-Reply-To: <xmqq4k7k8cz3.fsf@gitster.g>

Hi,

On Tue, 7 Dec 2021, Junio C Hamano wrote:

Show 14 quoted lines

> Josh Steadmon <steadmon@google.com> writes:
>
> > I've addressed feedback from V4. Since 2/3 reviewers seemed to (at least
> > slightly) prefer handling multiple upstream branches in the existing
> > tracking setup, I've gone that direction rather than repurposing the
> > branch copy code. None of the other issues were controversial.
> >
> > In this version, I'd appreciate feedback mainly on patch 1:
> > * Is the combination of `git_config_set_gently()` +
> >   `git_config_set_multivar_gently() the best way to write multiple
> >   config entries for the same key?
>
> IIRC git_config_set_*() is Dscho's brainchild.  If he is available
> to comment, it may be a valuable input.

The `git_config_set_multivar_gently()` function was really only intended to add one key/value pair.

Currently, there is no function to add multiple key/value pairs, and while it is slightly wasteful to lock the config multiple times to write a bunch of key/value pairs, it is not the worst in the world for a small use case like this one.

So yes, for the moment I would go with the suggested design.

One thing you might want to do is to avoid the extra `git_config_set_gently()` before the `for` loop, simply by passing `i == 0 ? 0 : CONFIG_FLAGS_MULTI_REPLACE` as `flags` parameter to the multivar version of the function.

But that would optimize for code size rather than for readability, and I would actually prefer the more verbose version.

Ciao, Dscho