Subject: Re: [PATCH 2/6] remote.c: drop "remote" pointer from "struct branch"

From: Jeff King <[email-masked]>
Date: Tue, 31 Mar 2015 18:24:31 -0400

On Tue, Mar 31, 2015 at 01:50:05PM -0700, Junio C Hamano wrote:

> > Getting rid of it drops one potential source of confusion:
> > is the value the match for "remote_name", or is it the
> > remote we would fetch from when on that branch (i.e., does
> > it fall back to "origin")?
> 
> I had to read the above three times before I realized that you were
> wondering "what is the value of this 'remote' field?  is it what
> remote_get() would give us for 'remote_name' and is NULL if
> remote_name is not set, or is it never NULL and instead have the
> remote for 'origin' if remote_name is not set?"
> 
> But perhaps it is just me.
> 
> We certainly have duplicated information between the two fields, and
> it first looked somewhat unnatural that you kept the name with which
> you need to trigger a search for the structure, instead of keeping
> the structure, one of whose field is its name already.  Perhaps
> there was a valid reason behind this choice, and I am guessing that
> it is probably because it will not let you differenciate the case
> where the user explicitly said 'origin' and we used 'origin' as a
> fallback, if you keep a "remote" field that stores the instance of
> the remote structure for 'origin' without keeping "remote_name".

That is the reason I was trying to explain above. Though I suppose you
could argue that remote_name suffers the same question (i.e., would we
ever set it to "origin"?)

It is much worse for pushremotes, which can come from
branch.*.pushremote, remote.pushdefault, branch.*.remote, or "origin".

I'll try to re-word the commit message.

-Peff