From: Marius Storm-Olsen <marius@trolltech.com>
Date: Mon, 02 Feb 2009 07:48:09 GMT
Subject: Re: [PATCH v2 1/4] Add log.mailmap as configurational option for mailmap location
Message-ID: <4986A539.3080606@trolltech.com>
In-Reply-To: <7vwsc9o82r.fsf@gitster.siamese.dyndns.org>

Junio C Hamano said the following on 02.02.2009 04:01:
> Marius Storm-Olsen <marius@trolltech.com> writes:
>> This allows us to override a repo mailmap file, or to use
>> mailmap files elsewhere than the repository root.
> 
> I think the new feature is not so well conceived.
> 
> Should it always be a wholesale override, or should it also support
> augmenting the repository version with a private copy?

Sure, I can rewrite it to be augmenting, rather than overriding.
I assume that the normal .mailmap file should be parsed first, then 
the log.mailmap one?


>> diff --git a/builtin-blame.c b/builtin-blame.c
>> index aae14ef..4b63775 100644
>> --- a/builtin-blame.c
>> +++ b/builtin-blame.c
>> @@ -2394,7 +2394,7 @@ parse_done:
>>  		die("reading graft file %s failed: %s",
>>  		    revs_file, strerror(errno));
>>  
>> -	read_mailmap(&mailmap, ".mailmap", NULL);
>> +	read_mailmap(&mailmap, NULL, NULL);
> 
> Your callers always seem to pass NULL for the second argument.  Doesn't it
> make a lot more sense to get rid of it?

Sure, but I left it in to allow "old-style" usage. Just in case the 
were use-cases for not using the log.mailmap one. I can nuke the 
arguments if you don't want them anymore.

-- 
.marius [@trolltech.com]
'if you know what you're doing, it's not research'