{"thread":{"id":"51462","subject":"[PATCH] log: use mailmap by default","startedAt":"2019-07-11T08:20:07Z","lastAt":"2019-07-11T17:43:07Z","messageCount":5,"participants":["Ariadne Conill","brian m. carlson","Martin Ågren"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"378797","messageId":"20190711081958.2201-1-ariadne@dereferenced.org","threadId":"51462","inReplyTo":null,"subject":"[PATCH] log: use mailmap by default","fromName":"Ariadne Conill","fromEmail":"ariadne@dereferenced.org","sentAt":"2019-07-11T08:19:58Z","receivedAt":"2019-07-11T08:20:07Z","isPatch":true,"sender":{"key":"ariadne@dereferenced.org","avatar":"https://avatars.githubusercontent.com/u/1522444?v=4"},"body":"The `git log` command shows the author and committer name recorded in\nthe git repository itself, while other commands respect `.mailmap`\nby default. I believe this is a bad design: it causes log entries to\nreflect inaccurate information: anyone who changes their name or\ne-mail address will not have that change (recorded in mailmap file)\nreflected when using `git log` by default.\n\nAnyone who explicitly wants the current behaviour can clearly request\nit by setting the `log.mailmap` setting to `false` in their\n`.gitconfig` file.\n\nSigned-off-by: Ariadne Conill \n---\n builtin/log.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 7c8767d3bc..3d2ce8fa3d 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -47,7 +47,7 @@ static int default_follow;\n static int default_show_signature;\n static int decoration_style;\n static int decoration_given;\n-static int use_mailmap_config;\n+static int use_mailmap_config = 1;\n static const char *fmt_patch_subject_prefix = \"PATCH\";\n static const char *fmt_pretty;\n \n-- \n2.17.1\n\n"},{"id":"378807","messageId":"20190711151900.GI9224@genre.crustytoothpaste.net","threadId":"51462","inReplyTo":"20190711081958.2201-1-ariadne@dereferenced.org","subject":"Re: [PATCH] log: use mailmap by default","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-07-11T15:19:00Z","receivedAt":"2019-07-11T15:19:09Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-07-11 at 08:19:58, Ariadne Conill wrote:\n> The `git log` command shows the author and committer name recorded in\n> the git repository itself, while other commands respect `.mailmap`\n> by default. I believe this is a bad design: it causes log entries to\n> reflect inaccurate information: anyone who changes their name or\n> e-mail address will not have that change (recorded in mailmap file)\n> reflected when using `git log` by default.\n> \n> Anyone who explicitly wants the current behaviour can clearly request\n> it by setting the `log.mailmap` setting to `false` in their\n> `.gitconfig` file.\n\nWhile I'm in favor of using the mailmap by default, typically we want a\nway people can override a default setting from the command line.\n\nSo in this case, we have \"--use-mailmap\", but we don't have a\n\"--no-use-mailmap\" (at least, it's not documented in the manpage). I\nthink we'd want to add such an option so that people can set it if they\nneed non-default behavior.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"378828","messageId":"CAAOiGNzXreYk9xoVOFkcivrXK4QkH495T7Lq76j3oz593rSzDQ@mail.gmail.com","threadId":"51462","inReplyTo":"20190711151900.GI9224@genre.crustytoothpaste.net","subject":"Re: [PATCH] log: use mailmap by default","fromName":"Ariadne Conill","fromEmail":"ariadne@dereferenced.org","sentAt":"2019-07-11T16:25:32Z","receivedAt":"2019-07-11T16:25:45Z","isPatch":true,"sender":{"key":"ariadne@dereferenced.org","avatar":"https://avatars.githubusercontent.com/u/1522444?v=4"},"body":"Hello,\n\nOn Thu, Jul 11, 2019 at 10:19 AM brian m. carlson\n wrote:\n>\n> On 2019-07-11 at 08:19:58, Ariadne Conill wrote:\n> > The `git log` command shows the author and committer name recorded in\n> > the git repository itself, while other commands respect `.mailmap`\n> > by default. I believe this is a bad design: it causes log entries to\n> > reflect inaccurate information: anyone who changes their name or\n> > e-mail address will not have that change (recorded in mailmap file)\n> > reflected when using `git log` by default.\n> >\n> > Anyone who explicitly wants the current behaviour can clearly request\n> > it by setting the `log.mailmap` setting to `false` in their\n> > `.gitconfig` file.\n>\n> While I'm in favor of using the mailmap by default, typically we want a\n> way people can override a default setting from the command line.\n>\n> So in this case, we have \"--use-mailmap\", but we don't have a\n> \"--no-use-mailmap\" (at least, it's not documented in the manpage). I\n> think we'd want to add such an option so that people can set it if they\n> need non-default behavior.\n\nI agree that there is probably some useful reasons to have this\noption, so I can add an option that forces mailmap usage off to\nsupplement the --use-mailmap option. It's no problem. I will\ngenerate a new patch series in a few minutes.\n\nThanks for reviewing!\n\nAriadne\n"},{"id":"378831","messageId":"CAN0heSqZcTBE4qtCNQNUVB=4y8OUh03U5_a+PDJU7vfZ9Nwu9w@mail.gmail.com","threadId":"51462","inReplyTo":"CAAOiGNzXreYk9xoVOFkcivrXK4QkH495T7Lq76j3oz593rSzDQ@mail.gmail.com","subject":"Re: [PATCH] log: use mailmap by default","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-07-11T17:09:30Z","receivedAt":"2019-07-11T17:09:45Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"Hi Ariadne,\n\nWelcome to the list!\n\nOn Thu, 11 Jul 2019 at 18:39, Ariadne Conill wrote:\n>\n> Hello,\n>\n> On Thu, Jul 11, 2019 at 10:19 AM brian m. carlson\n> wrote:\n> >\n> > On 2019-07-11 at 08:19:58, Ariadne Conill wrote:\n> > > The `git log` command shows the author and committer name recorded in\n> > > the git repository itself, while other commands respect `.mailmap`\n> > > by default. I believe this is a bad design: it causes log entries to\n> > > reflect inaccurate information: anyone who changes their name or\n> > > e-mail address will not have that change (recorded in mailmap file)\n> > > reflected when using `git log` by default.\n> > >\n> > > Anyone who explicitly wants the current behaviour can clearly request\n> > > it by setting the `log.mailmap` setting to `false` in their\n> > > `.gitconfig` file.\n\nIt would be useful with some tests for this. That would be a nice way to\nshowcase how this is meant to work, and to protect this from being\nbroken in the future. From looking around in t/, it looks like\nt4203-mailmap.sh could be a good spot.\n\n> > While I'm in favor of using the mailmap by default, typically we want a\n> > way people can override a default setting from the command line.\n> >\n> > So in this case, we have \"--use-mailmap\", but we don't have a\n> > \"--no-use-mailmap\" (at least, it's not documented in the manpage). I\n> > think we'd want to add such an option so that people can set it if they\n> > need non-default behavior.\n>\n> I agree that there is probably some useful reasons to have this\n> option, so I can add an option that forces mailmap usage off to\n> supplement the --use-mailmap option. It's no problem. I will\n> generate a new patch series in a few minutes.\n\nThere are some tests already in t4203 which test that for example `git\nlog --use-mailmap ...` does the right thing. If your implementation is\ncorrect, it should be possible to drop `--use-mailmap` from there and\neverything should work. It would then be very useful to have tests that\n`--no-use-mailmap` does the right thing. There's also some testing\naround the `log.mailmap` config, which you should probably adapt and\npossibly extend.\n\nThe documentation for `log.mailmap` will need to be updated, since\nright now it says (or at least implies) that the default is \"false\".\n\nThis looks like one of those very small and simple changes that turn\ninto quite some work to ensure that the tests are all there and that\nthe documentation is up to date. :-)\n\nCheers\n\nMartin\n"},{"id":"378841","messageId":"CAAOiGNxnOQ-FMGwBzZ92c8gJr0i+e8VBNCp6itJGCs24C6SGvg@mail.gmail.com","threadId":"51462","inReplyTo":"CAN0heSqZcTBE4qtCNQNUVB=4y8OUh03U5_a+PDJU7vfZ9Nwu9w@mail.gmail.com","subject":"Re: [PATCH] log: use mailmap by default","fromName":"Ariadne Conill","fromEmail":"ariadne@dereferenced.org","sentAt":"2019-07-11T17:42:54Z","receivedAt":"2019-07-11T17:43:07Z","isPatch":true,"sender":{"key":"ariadne@dereferenced.org","avatar":"https://avatars.githubusercontent.com/u/1522444?v=4"},"body":"Hello,\n\nOn Thu, Jul 11, 2019 at 12:09 PM Martin Ågren wrote:\n>\n> Hi Ariadne,\n>\n> Welcome to the list!\n\nThanks!\n\n> On Thu, 11 Jul 2019 at 18:39, Ariadne Conill wrote:\n> >\n> > Hello,\n> >\n> > On Thu, Jul 11, 2019 at 10:19 AM brian m. carlson\n> > wrote:\n> > >\n> > > On 2019-07-11 at 08:19:58, Ariadne Conill wrote:\n> > > > The `git log` command shows the author and committer name recorded in\n> > > > the git repository itself, while other commands respect `.mailmap`\n> > > > by default. I believe this is a bad design: it causes log entries to\n> > > > reflect inaccurate information: anyone who changes their name or\n> > > > e-mail address will not have that change (recorded in mailmap file)\n> > > > reflected when using `git log` by default.\n> > > >\n> > > > Anyone who explicitly wants the current behaviour can clearly request\n> > > > it by setting the `log.mailmap` setting to `false` in their\n> > > > `.gitconfig` file.\n>\n> It would be useful with some tests for this. That would be a nice way to\n> showcase how this is meant to work, and to protect this from being\n> broken in the future. From looking around in t/, it looks like\n> t4203-mailmap.sh could be a good spot.\n\nI agree. I added a regression test to t4203-mailmap.sh in my second\ngo around, already.\n\n> > > While I'm in favor of using the mailmap by default, typically we want a\n> > > way people can override a default setting from the command line.\n> > >\n> > > So in this case, we have \"--use-mailmap\", but we don't have a\n> > > \"--no-use-mailmap\" (at least, it's not documented in the manpage). I\n> > > think we'd want to add such an option so that people can set it if they\n> > > need non-default behavior.\n> >\n> > I agree that there is probably some useful reasons to have this\n> > option, so I can add an option that forces mailmap usage off to\n> > supplement the --use-mailmap option. It's no problem. I will\n> > generate a new patch series in a few minutes.\n>\n> There are some tests already in t4203 which test that for example `git\n> log --use-mailmap ...` does the right thing. If your implementation is\n> correct, it should be possible to drop `--use-mailmap` from there and\n> everything should work. It would then be very useful to have tests that\n> `--no-use-mailmap` does the right thing. There's also some testing\n> around the `log.mailmap` config, which you should probably adapt and\n> possibly extend.\n\nAlright, I'll adapt the tests as proposed. I agree with your\nconclusion that dropping `--use-mailmap` should be perfectly safe and\nnot cause any regressions in the testsuite.\n\nI already did add a regression test for `--no-use-mailmap` which tests\nit with `-c log.mailmap=True`.\n\n> The documentation for `log.mailmap` will need to be updated, since\n> right now it says (or at least implies) that the default is \"false\".\n\nYeah, I just noticed the documentation needs to be updated for this\nchange. That will be resolved in v3, as well.\n\n> This looks like one of those very small and simple changes that turn\n> into quite some work to ensure that the tests are all there and that\n> the documentation is up to date. :-)\n\nYeah, this is definitely more docs and test oriented I think. Thanks\nfor the review.\n\nAriadne\n"}]}