Show 36 quoted lines
> Hi,
>
> On Tue, 7 Dec 2021, Junio C Hamano wrote:
>
> > 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.