Jared Hance <jaredhance@xxxxxxxxx> writes: > Add the option merge.defaultupstream to add support for merging from > the upstream branch by default. The upstream branch is found using > branch.[name].merge. > > Signed-off-by: Jared Hance <jaredhance@xxxxxxxxx> > --- Thanks; the first three in the series looks right. > diff --git a/Documentation/config.txt b/Documentation/config.txt > index c5e1835..4415691 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -1389,6 +1389,12 @@ man.<tool>.path:: > > include::merge-config.txt[] > > +merge.defaultUpstream:: I somehow had an impression that majority of others convinced you to rename this to default-to-upstream, but I may be mistaken. I think somebody who does not know the history of the development and discussion of this feature would think that this specifies the default upstream remote (i.e. not a boolean variable, but a string) given this name. > @@ -536,7 +539,9 @@ static int git_merge_config(const char *k, const char *v, void *cb) > if (status <= 0) > return status; > > - if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat")) > + if (!strcmp(k, "merge.defaultupstream")) > + default_upstream = git_config_bool(k, v); > + else if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat")) > show_diffstat = git_config_bool(k, v); It is somewhat rude to reviewers' eyes to turn an existing "if" into "else if" and place new stuff at the beginning; unless there is a good reason that the new stuff has to be at the beginning, that is. This is not a new issue, but a callback function to git_config() should return once it recognized and handled the variable, instead of cascading the controll out. The "diffstat/stat" code shows a bad example and you inherited the badness from there. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html