From: Matthieu Moy Date: Thu, 18 Apr 2019 09:51:28 GMT Subject: Re: [PATCH] [WIP/RFC] add git pull and git fetch --set-upstream Message-ID: <86h8av7ian.fsf@univ-lyon1.fr> In-Reply-To: <36559daca9d84f7a91933add734020cd@BPMBX2013-01.univ-lyon1.fr> Junio C Hamano writes: >> --- a/Documentation/fetch-options.txt >> +++ b/Documentation/fetch-options.txt >> @@ -165,6 +165,11 @@ ifndef::git-pull[] >> Disable recursive fetching of submodules (this has the same effect as >> using the `--recurse-submodules=no` option). >> >> +--set-upstream:: >> + If the new URL remote is correct, pull and add upstream (tracking) >> + reference, used by argument-less linkgit:git-push[1] and other commands. > > git-push and other commands? I think this is taken from the documentation of --set-upstream for push, which says: -u:: --set-upstream:: For every branch that is up to date or successfully pushed, add upstream (tracking) reference, used by argument-less linkgit:git-pull[1] and other commands. For more information, see `branch..merge` in linkgit:git-config[1]. Probably the reasoning was to make a symmetry between "git push --set-upstream", which mentions "pull" in the doc, and the new "git pull --set-upstream". However, I do not think there should be such symmetry: Actually, the way I see it, the notion of uptream (i.e. branch..remote and branch..merge) is primarily about "pull" and friends, and "push" happens to use it also by default. But when branch..pushRemote is set, upstream is really about pulling, and pushing goes to the pushRemote. >> + /* TODO: remove debug trace */ > > Perhaps do so before sending it out for the review? Yes. This is WIP for now, but it's time to get closer to a real patch, and these debug statements are counter-productive for that. >> + test_must_be_empty merge.$1 >> +} > > If this wanted to say "It is OK for the variable to be missing, and > it also is OK for the variable to have an empty string as its value; > all other cases are unacceptable", Actually, I don't think the "present but empty" case makes sense here, so just test_must_fail git config "$1" should do the trick. I agree with all other remarks. -- Matthieu Moy https://matthieu-moy.fr/