From mboxrd@z Thu Jan 1 00:00:00 1970 From: john at keeping.me.uk (John Keeping) Date: Sat, 28 Oct 2017 12:36:46 +0100 Subject: [PATCH 0/3] Add support for git's mailmap. In-Reply-To: <2219e113-66c7-e0a1-664a-d160d43449d6@bnl.gov> References: <7b18e5bf-635b-7773-e352-c9a54ab46a20@bnl.gov> <2219e113-66c7-e0a1-664a-d160d43449d6@bnl.gov> Message-ID: <20171028113646.GG2393@john.keeping.me.uk> On Tue, Oct 24, 2017 at 10:55:21AM -0400, Jason A. Smith wrote: > Seeing a lot of recent activity I checked the upstream and see that the > patch I first submitted over a year ago has still not been merged. I > already had to rebase it once several months ago to fix conflicts, I > don't want to have to do that again. Is there something wrong with this > patch? Please let me know so I can fix it. I've taken another look at this patch and one thing that stands out is that we end up inserting code to map users in a lot of different places. I haven't audited the code, but after the final patch, is there anywhere that calls cgit_parse_commit() and doesn't go on to map the users? (Even if not all callers do map the user, can we push cgit_map_user() into cgit_parse_commit() and use a flag parameter to enable it?) But I think that points to a deeper philosophical question of whether we should be mapping the user in all cases (and I suspect this is why the patch has sat on the list for so long - there are arguments in both directions). As I understand it, mailmap was originally introduced so that git-shortlog would coalesce commits from the same author but with different email addresses. When using the git command, mailmap is enabled by default for git-shortlog and git-blame but not for git-log, git-show, and other commands that print commits. Extending this to CGit, it seems that there's a clear argument for enabling mailmap in ui-stats and possibly the new ui-blame, but when we print a commit in ui-commit or ui-log is it always correct to apply the mailmap or should we show the commit headers as-is? It would be interesting to know what others on the list think about this. If there isn't a consensus then we may want a new config option to allow this to be enabled according to the preferences of individual sites.