Re: [PATCH 4/5] remote.c: teach branch_get() to treat symrefs other than HEAD

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: May 1, 2013, 19:28 UTC
Message-ID: <CAMP44s34X+6c2V0iU_+dfNR9ULXD+Q0CDFk0S9qdK0n8AC81DQ@mail.gmail.com>
In-Reply-To: <CALkWK0n2odCn=GnYSXv7g113PFEX42WF1d2GBGV=ye2TuY3CHA@mail.gmail.com>

On Wed, May 1, 2013 at 1:44 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

Show 6 quoted lines

> Felipe Contreras wrote:
>> But why? I'm not familiar with branch_get, but my intuition tells me
>> you are changing the behavior, and now branch_get() is doing something
>> it wasn't intended to do. And for what?
>
> Why is there a commit message?  I've explained what the behavior change is.

Not good enough.

Show 7 quoted lines

>> Your rationale is that it fixes the test cases below, but that's not
>> reason enough, since there are other ways to fix them, as my patch
>> series shows.
>
> For what exactly.  To fix a real bug: H@{u} and @@{u} don't work where
> either H or @ are symbolic refs.  I want custom symbolic refs, because
> they are useful.  In other words, "HEAD" is not a sacred symbolic ref.

As I said, the @@{u} thing can be fixed through other ways.

Moreover, "HEAD" is still a special case in remote.c::branch_get() that you just modified.

>> I think these are two patches should be introduced separately, and
>> with a reason for them to exist independent of each other.
>
> I cannot justify the remote.c patch without the "@{" change.

That's what I thought.

-- 
Felipe Contreras