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

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

quoted

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