Re: [PATCH v2] Add push --set-upstream

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 15, 2010, 23:40 UTC
Message-ID: <7vzl4frl7i.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1263595630-18962-1-git-send-email-ilari.liusvaara@elisanet.fi>

Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:

Show 8 quoted lines

> @@ -974,6 +1016,10 @@ int transport_push(struct transport *transport,
>  	verify_remote_names(refspec_nr, refspec);
>  
>  	if (transport->push) {
> +		/* Maybe FIXME. But no important transport uses this case. */
> +		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
> +			die("This transport does not support using --set-upstream");
> +

Would it be better to just warn() and continue instead of dying? I think it can be argued both ways, and I personally think die() is better by making it more visible that the user does not have the config s/he wanted to add, but I am pointing it out just in case somebody thinks of a better solution (of course, doing an extra ls-remote and doing the configuration is such a "better solution" but that is not what I mean---I am not that greedy).

Show 10 quoted lines

> @@ -1002,6 +1048,9 @@ int transport_push(struct transport *transport,
>  					verbose | porcelain, porcelain,
>  					nonfastforward);
>  
> +		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
> +			set_upstreams(transport, remote_refs);
> +
>  		if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
>  			struct ref *ref;
>  			for (ref = remote_refs; ref; ref = ref->next)

Shouldn't this honor TRANSPORT_PUSH_DRY_RUN? IOW, when should it touch the configuration if you do this sequence?

	# I am paranoid and want to check what happens first
	git push -n --track there this
        # Ok let's do it for real.
        git push    --track there this