Junio C Hamano <[email protected]> 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.<name>.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.<branch>.remote and branch.<branch>.merge) is primarily about
"pull" and friends, and "push" happens to use it also by default. But
when branch.<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/
