On 2021.12.10 23:48, Johannes Schindelin wrote: > Hi, > > On Tue, 7 Dec 2021, Junio C Hamano wrote: > > > Josh Steadmon <steadmon@xxxxxxxxxx> 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. Sounds good, thanks for the advice! > Ciao, > Dscho
