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. Ciao, Dscho
