* [PATCH 0/3] Add support for git's mailmap. @ 2017-02-24 16:18 smithj4 2017-10-24 14:55 ` smithj4 0 siblings, 1 reply; 7+ messages in thread From: smithj4 @ 2017-02-24 16:18 UTC (permalink / raw) Rebased patch to resolve some minor conflicts with the master branch. Jason A. Smith (1): Add support for git's mailmap. John Keeping (2): filter: introduce cgit_open_email_filter() wrapper Remove angle brackets from {author,committer}_email cgit.h | 5 +++++ filter.c | 13 +++++++++++++ parsing.c | 27 ++++++++++++++++++++++----- ui-atom.c | 31 ++++++++++++++----------------- ui-commit.c | 28 ++++++++++++++++++++-------- ui-log.c | 11 +++++++++-- ui-refs.c | 32 ++++++++++++++++++++++++++------ ui-stats.c | 13 ++++++++++--- ui-tag.c | 17 ++++++++++++----- 9 files changed, 131 insertions(+), 46 deletions(-) ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 0/3] Add support for git's mailmap. 2017-02-24 16:18 [PATCH 0/3] Add support for git's mailmap smithj4 @ 2017-10-24 14:55 ` smithj4 2017-10-28 11:36 ` john 0 siblings, 1 reply; 7+ messages in thread From: smithj4 @ 2017-10-24 14:55 UTC (permalink / raw) 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. Thanks, ~Jason On 02/24/2017 11:18 AM, Jason A. Smith wrote: > > Rebased patch to resolve some minor conflicts with the master branch. > > Jason A. Smith (1): > ? Add support for git's mailmap. > > John Keeping (2): > ? filter: introduce cgit_open_email_filter() wrapper > ? Remove angle brackets from {author,committer}_email > > ?cgit.h????? |? 5 +++++ > ?filter.c??? | 13 +++++++++++++ > ?parsing.c?? | 27 ++++++++++++++++++++++----- > ?ui-atom.c?? | 31 ++++++++++++++----------------- > ?ui-commit.c | 28 ++++++++++++++++++++-------- > ?ui-log.c??? | 11 +++++++++-- > ?ui-refs.c?? | 32 ++++++++++++++++++++++++++------ > ?ui-stats.c? | 13 ++++++++++--- > ?ui-tag.c??? | 17 ++++++++++++----- > ?9 files changed, 131 insertions(+), 46 deletions(-) > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 0/3] Add support for git's mailmap. 2017-10-24 14:55 ` smithj4 @ 2017-10-28 11:36 ` john 2017-11-02 20:48 ` smithj4 0 siblings, 1 reply; 7+ messages in thread From: john @ 2017-10-28 11:36 UTC (permalink / raw) 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 0/3] Add support for git's mailmap. 2017-10-28 11:36 ` john @ 2017-11-02 20:48 ` smithj4 2017-11-02 22:54 ` i 2017-11-05 12:25 ` john 0 siblings, 2 replies; 7+ messages in thread From: smithj4 @ 2017-11-02 20:48 UTC (permalink / raw) Hi John, I always thought that by going through the trouble of collecting the duplicate email addresses and creating the .mailmap file that you wanted this coalescing behavior. Personally, I don't really want to have to remember everyone's old email address and variation of their name that they might have used several years ago to push a commit. I don't really care what email & name they used a long time ago, I am more interested in just knowing who made the commit, which is why the .mailmap coalesces all of their old identities into their current one for display. If people don't need or want this coalescing behavior and want to see each individual identity then they don't need to make the .mailmap file to begin with. Just my opinion on .mailmap. ~Jason On 10/28/2017 07:36 AM, John Keeping wrote: > 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. > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 0/3] Add support for git's mailmap. 2017-11-02 20:48 ` smithj4 @ 2017-11-02 22:54 ` i 2017-11-05 12:25 ` john 1 sibling, 0 replies; 7+ messages in thread From: i @ 2017-11-02 22:54 UTC (permalink / raw) Ack and for edge cases a config entry ignore_mailmap = True On 02.11.2017 21:48, Jason A. Smith wrote: > If people don't need or want this coalescing behavior and want to see > each individual identity then they don't need to make the .mailmap file > to begin with. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: OpenPGP digital signature URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20171102/9cccb9fa/attachment.asc> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 0/3] Add support for git's mailmap. 2017-11-02 20:48 ` smithj4 2017-11-02 22:54 ` i @ 2017-11-05 12:25 ` john 1 sibling, 0 replies; 7+ messages in thread From: john @ 2017-11-05 12:25 UTC (permalink / raw) On Thu, Nov 02, 2017 at 04:48:08PM -0400, Jason A. Smith wrote: > I always thought that by going through the trouble of collecting the > duplicate email addresses and creating the .mailmap file that you wanted > this coalescing behavior. > > Personally, I don't really want to have to remember everyone's old email > address and variation of their name that they might have used several > years ago to push a commit. I don't really care what email & name they > used a long time ago, I am more interested in just knowing who made the > commit, which is why the .mailmap coalesces all of their old identities > into their current one for display. > > If people don't need or want this coalescing behavior and want to see > each individual identity then they don't need to make the .mailmap file > to begin with. Consider someone working on an open source project who changes employer; in this case .mailmap allows email to be directed to an address at the new employer rather than at the previous employer, especially for auto-generated CC lists from something like Linux's get_maintainer.pl script. However, when looking at historic commits, it may be very interesting that a particular change was committed by someone with an email address at a cerain company. Blindly applying .mailmap rules loses that information (admittedly, in the case of the kernel, DCO rules mean the relevant information also lives in the S-o-b but that doesn't apply everywhere). On the other hand, it might be that some early commits by a developer in a project use the default "user@(none)" fallback address and .mailmap is used to change that to a genuine email address. In that case applying .mailmap in all cases is desirable. I don't think we can take a global decision on which behaviour is correct for all projects, so IMHO there must be a configuration switch to allow projects to decide what is right for them. > On 10/28/2017 07:36 AM, John Keeping wrote: > > 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 0/3] Add support for git's mailmap. @ 2017-02-25 16:12 smithj4 0 siblings, 0 replies; 7+ messages in thread From: smithj4 @ 2017-02-25 16:12 UTC (permalink / raw) Sorry, I was using imap-send and used attachments to keep my thunderbird mail client from line-wrapping the patches. Jason A. Smith (1): Add support for git's mailmap. John Keeping (2): filter: introduce cgit_open_email_filter() wrapper Remove angle brackets from {author,committer}_email cgit.h | 5 +++++ filter.c | 13 +++++++++++++ parsing.c | 27 ++++++++++++++++++++++----- ui-atom.c | 31 ++++++++++++++----------------- ui-commit.c | 28 ++++++++++++++++++++-------- ui-log.c | 11 +++++++++-- ui-refs.c | 32 ++++++++++++++++++++++++++------ ui-stats.c | 13 ++++++++++--- ui-tag.c | 17 ++++++++++++----- 9 files changed, 131 insertions(+), 46 deletions(-) -- 2.9.3 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-11-05 12:25 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-24 16:18 [PATCH 0/3] Add support for git's mailmap smithj4 2017-10-24 14:55 ` smithj4 2017-10-28 11:36 ` john 2017-11-02 20:48 ` smithj4 2017-11-02 22:54 ` i 2017-11-05 12:25 ` john 2017-02-25 16:12 smithj4
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).