Re: [PATCH] [WIP/RFC] add git pull and git fetch --set-upstream

[Matthieu Moy <Matthieu.Moy@univ-lyon1.fr>]

Junio C Hamano <gitster@pobox.com> 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/