Re: [PATCH 1/7] add special "matching refs" refspec

From: Paolo Bonzini <bonzini@gnu.org>
Date: Apr 30, 2008, 09:35 UTC
Message-ID: <48183D44.8080507@gnu.org>
In-Reply-To: <7vfxt31ys6.fsf@gitster.siamese.dyndns.org>

Show 9 quoted lines

>> +		if (rs[i].matching &&
>> +		    (matching_refs == -1 || rs[i].force)) {
>> +			matching_refs = i;
>> +			continue;
>> +		}
>
> It is probably better to document that you would force if you have both
> "+:" and ":" for the same remote, even though I am not sure if allowing
> that (instead of diagnosing it as an error) is the right thing to do.

I screwed up here.  I was sure that something like

         push = refs/heads/*:refs/heads/*
         push = +refs/heads/*:refs/heads/*

would also force, instead the first one wins. I'm ok with just removing the "|| rs[i].force" part.

> Is it an error to have both ":" and "some:other" refspecs for the same
> remote?  If so who makes the check?

No, it is not an error.  For example, it allows to have a

	push = refs/tags/*:refs/tags/*
	push = :

refspec, which pushes all tags but only pushes branches if there is a matching ref on the other side. I don't have a use for this, but it made sense to support the general case.

> Otherwise this patch seems to be very cleanly done.  Especially I like how
> the updated match_refs() looks.

Thanks.  Should I resubmit?

Paolo