Re: [PATCH] git push --track

From: Rudolf Polzer <divverent@alientrap.org>
Date: Jan 15, 2010, 14:00 UTC
Message-ID: <20100115140048.GB30986@rm.endoftheinternet.org>
In-Reply-To: <7vvdf33onp.fsf@alter.siamese.dyndns.org>

On Thu, Jan 14, 2010 at 09:47:22PM -0800, Junio C Hamano wrote:

Show 15 quoted lines

> I have a feeling that it is more appropriate to have the additional code
> in transport_push(), which gets ls-remote information, runs match_refs()
> and finally calls transport->push_refs().  I think the extra branch
> configuration would fit better inside the if block immediately after all
> that happens, i.e.
> 
> 	if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
> 		struct ref *ref;
> 		for (ref = remote_refs; ref; ref = ref->next)
> 			update_tracking_ref(transport->remote, ref, verbose);
> +		if (flags & TRANSPORT_PUSH_RECONFIGURE_FORK)
> +			configure_forked_branch(...);
> 	}
> 
> in transport.c

I thought about this place when making my patch, but didn't put it there because this function is not called in the rsync protocol (which defines transport->push). So I instead did the logical step and went into the caller of that function. Functionality-wise, it also isn't really a "transport" function but part of pushing.

Show 5 quoted lines

> > +		if(!(flags & TRANSPORT_PUSH_DRY_RUN))
> > +		if(!match_refs(local_refs, &remote_refs, refspec_nr, refspec, match_flags))
> 
> Yuck; hiding the fact that you have an over-nested logic is not a way to
> fix it.

This was accidental.

But well. Why bother with this, if this feature was rejected before already anyway.

Rudolf