Re: [PATCH 0/5] Extend pattern refspecs

From: Daniel Barkalow <barkalow@iabervon.org>
Date: Mar 6, 2009, 07:03 UTC
Message-ID: <alpine.LNX.1.00.0903060153490.19665@iabervon.org>
In-Reply-To: <76718490903052252y1778aa41g8f3e52329f7bf288@mail.gmail.com>

On Fri, 6 Mar 2009, Jay Soffian wrote:

Show 16 quoted lines

> On Fri, Mar 6, 2009 at 1:07 AM, Daniel Barkalow <barkalow@iabervon.org> wrote:
> > On Fri, 6 Mar 2009, Jay Soffian wrote:
> >
> > Actually, you should be able to just drop your "buf" and use spec->src and
> > spec->dst, since it just stores the original strings. So that should be
> > easy enough, although it might be good to go through a remote.c function
> > just in case it becomes more complicated later. On the other hand,
> > get_head_names() should probably get a patch like my 1/5 to have it use
> > the remote.c parser, or should use a constant "head mirror" refspec like
> > that tag_refspec already in remote.c
> 
> Okay.
> 
> > Do you have tests for "git remote show -n"?
> 
> Yes. Apparently not enough of them though if nothing is failing.

It only seems to be off by saying:

  Local ref configured for 'git push' (status not queried):
    refs/heads/** forces to refs/heads/**

so you didn't necessarily miss much, just the one thing I seem to have broken.

Show 16 quoted lines

> > Merging my series (on top of
> > origin/master) and e5dcbfd and adding a final '*' to the string in
> > get_head_names() made everything pass for me, without doing anything about
> > the extra *s, but the output is clearly not quite right.
> 
> Hmm, alright.
> 
> > I'm not seeing anything that makes assumptions about the matching
> > semantics of pattern refspecs, just stuff about how the stored form
> > relates to the config-file form.
> 
> Okay, that sounds right.
> 
> I assume your series will end up in pu soon enough, and I think my
> series is about to hop to next. What's the right way to to have them
> be happy together?

The only "correctness of outcome" issue is the open-coded refspec initialization, I think, which is probably actually cleaner to have as a constant in remote.c anyway (unlike in builtin-clone, there's no variability at all, so it might as well be a constant.

I can amend my series to avoid adding the extra * in the message when your series graduates, and it should be clean enough to deal with if your series wound up getting dropped later; it'll be the only change in that file for my series, so I'd be able to drop it easily.

It'd be useful to have that message tested by your series, though, so I can verify my series reliably without worrying about whether I accidentally dropped both the fix and the test.

	-Daniel
*This .sig left intentionally blank*