From: Felipe Contreras Date: Mon, 06 Feb 2012 23:11:01 GMT Subject: Re: [bug] blame duplicates trailing ">" in mailmapped emails Message-ID: In-Reply-To: <7vliof62ko.fsf@alter.siamese.dyndns.org> On Tue, Feb 7, 2012 at 12:04 AM, Junio C Hamano wrote: > Felipe Contreras writes: > >> This subject doesn't explain the *purpose* of the patch: always return >> a plain mail address from map_user() > > That would be a much better subject. > >> I think the immediate problem should be here: >> >> Currently 'git blame -e' would add an extra '>' if map_user() returns >> true, which would end up as '>'. This is because >> map_user() sometimes modifies, the mail string, but sometimes not. So >> let's always modify it. > > That is just a symptom. That's a matter of semantics. Is the API broken? Then the API has a problem, and you are fixing it, otherwise you are merely *improving* it. > People who reached this commit by digging the > history of mailmap.c would need to see the *cause* of the symptom > described in the light of how the API is designed to be used. Well, when you have a summary as "always return a plain mail address from map_user()", I think it's pretty clear what is the "problem"; the API does not always return a plain mail address. But any case, that is described below, in my suggestion. Besides, even people digging the history would benefit from seeing the "symptom" first. > In other > words, "the code after the update has to be this way because these are the > i/o constraints this API has".  "Otherwise you would see this breakage for > example" is merely a supporting material. I disagree. The reader of the current commit message will keep in his/her head the question "Why?", and it would be answered only at the very end. Besides, what more succinct way to describe a problem with the API than with an example (that happens to be real). -- Felipe Contreras