Thread (4 messages) flat view 4 messages, 3 authors, 2016-06-15

Re: [PATCH] branch.autosetupmerge: allow boolean values, or "all"

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:20

Possibly related (same subject, not in this thread)

Hi,

On Sun, 8 Jul 2007, Paolo Bonzini wrote:
[Paolo tried to hide the fact that it was Junio who wrote this:]
quoted
Having prepared the patch below, I do not think if the original patch 
even wanted to have 'all' semantics.  The surrounding text only talks 
about "off a remote branch" and I strongly suspect that nobody wanted 
to do this for a local branch case at all.
If I remember correctly, the problem was that you are not sure that 
remote branches are in refs/remotes.
Then you code is incorrect.

Basically, you use a confusing set of four functions to do the following:

- read the config, and
- write the branch.<name>.{remote,merge} variables

Two functions would have been sufficient, and easier to read.  And as I 
fully expect with non-simple code, a bug was lurking.  This time in 
set_branch_defaults():

you check if neither config_repo nor config_remote (which is a misnomer, 
as it does not contain a "remote", but a "remote branch") is set.  But 
that happens when there was no information in the config, too!

Also you miss the case that there is ambiguous information:

[remote "hello"]
	url = git://blub/x.git
	fetch = refs/heads/master:refs/heads/origin

[remote "bello"]
	url = git://yaddayadda/x.git
	fetch = refs/heads/master:refs/heads/origin

See? Your code just uses "bello".

Will send out a fix shortly.

Ciao,
Dscho
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help