Re: [PATCH/RFC] git-svn: don't create master if another head exists

[Junio C Hamano <gitster@pobox.com>]

Eric Wong <normalperson@yhbt.net> writes:

> Marcin Owsiany <marcin@owsiany.pl> wrote:
>> git-svn insists on creating the "master" head (unless it exists) on every
>> "fetch". While it is useful that it gets created initially (users expect this
>> git convention), some users find it annoying that it gets recreated, especially
>> when they would like the git branch names to follow SVN repository branch
>> names. More background in
>> http://thread.gmane.org/gmane.comp.version-control.git/115030
>> 
>> Make git-svn skip the "master" creation if there is another head ref pointing
>> to the same place. This means "master" does get created on initial "clone" but
>> does not get recreated once a user deletes it.
>
> Sounds reasonable to me.  Thanks for following up on this after all this
> time :)
>
>> Signed-off-by: Marcin Owsiany <marcin@owsiany.pl>
>> ---
>>  git-svn.perl |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>> 
>> diff --git a/git-svn.perl b/git-svn.perl
>> index 0b074c4..90f3d06 100755
>> --- a/git-svn.perl
>> +++ b/git-svn.perl
>> @@ -1613,6 +1613,8 @@ sub post_fetch_checkout {
>>  	}
>>  
>>  	my $valid_head = verify_ref('HEAD^0');
>> +	my @heads_commits = eval { command(qw(show-ref --heads --hash)) };
>> +	return if $valid_head and grep { $_ eq $valid_head } @heads_commits;
>
> I (and I believe most git hackers) prefer C-style "&&" for boolean
> expressions:
>
> 	return if $valid_head && grep { $_ eq $valid_head } @heads_commits;
>
> "and" is lower precedence and best reserved for control flow:
>
> 	-f $file and print "File: $file exists!\n";
>
> There's no logical difference in this case, but "&&" is probably easier for
> C programmers to read.
>
> I'll just swap "and" for "&&" and push unless there's any objection from
> you.

I personally do not like either (I would generally avoid the
statement modifiers unless the condition is really trivial and it is
about an exceptional case) from the style point of view, but more
importantly, I do not understand what the new check is trying to do,
and I am not convinced if it is the whole solution to "I do not want
'master', as I am using something else" problem (please see my two
review messages).