{"thread":{"id":"44277","subject":"Allow \"git shortlog\" to group by committer information","startedAt":"2016-10-11T18:46:15Z","lastAt":"2016-12-21T21:10:02Z","messageCount":24,"participants":["Linus Torvalds","Jeff King","Junio C Hamano","Stephen & Linda Smith","Johannes Sixt","Jacob Keller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"303902","messageId":"CA+55aFzWkE43rSm-TJNKkHq4F3eOiGR0-Bo9V1=a1s=vQ0KPqQ@mail.gmail.com","threadId":"44277","inReplyTo":null,"subject":"Allow \"git shortlog\" to group by committer information","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2016-10-11T18:45:58Z","receivedAt":"2016-10-11T18:46:15Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"In some situations you may want to group the commits not by author,\nbut by committer instead.\n\nFor example, when I just wanted to look up what I'm still missing from\nlinux-next in the current merge window, I don't care so much about who\nwrote a patch, as what git tree it came from, which generally boils\ndown to \"who committed it\".\n\nSo make git shortlog take a \"-c\" or \"--committer\" option to switch\ngrouping to that.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n\n\n builtin/shortlog.c | 15 ++++++++++++---\n shortlog.h         |  1 +\n 2 files changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex ba0e1154a..c9585d475 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -117,11 +117,15 @@ static void read_from_stdin(struct shortlog *log)\n {\n \tstruct strbuf author = STRBUF_INIT;\n \tstruct strbuf oneline = STRBUF_INIT;\n+\tstatic const char *author_match[2] = { \"Author: \", \"author \" };\n+\tstatic const char *committer_match[2] = { \"Commit: \", \"committer \" };\n+\tconst char **match;\n \n+\tmatch = log->committer ? committer_match : author_match;\n \twhile (strbuf_getline_lf(&author, stdin) != EOF) {\n \t\tconst char *v;\n-\t\tif (!skip_prefix(author.buf, \"Author: \", &v) &&\n-\t\t    !skip_prefix(author.buf, \"author \", &v))\n+\t\tif (!skip_prefix(author.buf, match[0], &v) &&\n+\t\t    !skip_prefix(author.buf, match[1], &v))\n \t\t\tcontinue;\n \t\twhile (strbuf_getline_lf(&oneline, stdin) != EOF &&\n \t\t       oneline.len)\n@@ -140,6 +144,7 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \tstruct strbuf author = STRBUF_INIT;\n \tstruct strbuf oneline = STRBUF_INIT;\n \tstruct pretty_print_context ctx = {0};\n+\tconst char *fmt;\n \n \tctx.fmt = CMIT_FMT_USERFORMAT;\n \tctx.abbrev = log->abbrev;\n@@ -148,7 +153,9 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \tctx.date_mode.type = DATE_NORMAL;\n \tctx.output_encoding = get_log_output_encoding();\n \n-\tformat_commit_message(commit, \"%an <%ae>\", &author, &ctx);\n+\tfmt = log->committer ? \"%cn <%ce>\" : \"%an <%ae>\";\n+\n+\tformat_commit_message(commit, fmt, &author, &ctx);\n \tif (!log->summary) {\n \t\tif (log->user_format)\n \t\t\tpretty_print_commit(&ctx, commit, &oneline);\n@@ -238,6 +245,8 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \tint nongit = !startup_info->have_repository;\n \n \tconst struct option options[] = {\n+\t\tOPT_BOOL('c', \"committer\", &log.committer,\n+\t\t\t N_(\"Group by committer rather than author\")),\n \t\tOPT_BOOL('n', \"numbered\", &log.sort_by_number,\n \t\t\t N_(\"sort output according to the number of commits per author\")),\n \t\tOPT_BOOL('s', \"summary\", &log.summary,\ndiff --git a/shortlog.h b/shortlog.h\nindex 5a326c686..5d64cfe92 100644\n--- a/shortlog.h\n+++ b/shortlog.h\n@@ -13,6 +13,7 @@ struct shortlog {\n \tint in2;\n \tint user_format;\n \tint abbrev;\n+\tint committer;\n \n \tchar *common_repo_prefix;\n \tint email;\n"},{"id":"303908","messageId":"20161011190103.fovcwsze77hkew4t@sigill.intra.peff.net","threadId":"44277","inReplyTo":"CA+55aFzWkE43rSm-TJNKkHq4F3eOiGR0-Bo9V1=a1s=vQ0KPqQ@mail.gmail.com","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-11T19:01:03Z","receivedAt":"2016-10-11T19:03:41Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 11, 2016 at 11:45:58AM -0700, Linus Torvalds wrote:\n\n> In some situations you may want to group the commits not by author,\n> but by committer instead.\n> \n> For example, when I just wanted to look up what I'm still missing from\n> linux-next in the current merge window, I don't care so much about who\n> wrote a patch, as what git tree it came from, which generally boils\n> down to \"who committed it\".\n> \n> So make git shortlog take a \"-c\" or \"--committer\" option to switch\n> grouping to that.\n\nI made a very similar patch as part of a larger series:\n\n  http://public-inbox.org/git/20151229073515.GK8842@sigill.intra.peff.net/\n\nbut never followed through with it because it wasn't clear that grouping\nby anything besides author was actually useful to anybody.\n\nMy implementation is a little more complicated because it's also setting\nthings up for grouping by trailers (so you can group by \"signed-off-by\",\nfor example). I don't know if that's useful to your or not.\n\nI'm fine with this less invasive version, but a few suggestions:\n\n - do you want to call it --group-by=committer (with --group-by=author\n   as the default), which could later extend naturally to other forms of\n   grouping?\n\n - you might want to steal the tests and documentation from my patch\n   (though obviously they would need tweaked to match your interface)\n\n-Peff\n"},{"id":"303909","messageId":"CA+55aFzw24pHGOYFBFVvTbU1Cudcr8zcPt_RvdQSxrKY5weCbQ@mail.gmail.com","threadId":"44277","inReplyTo":"20161011190103.fovcwsze77hkew4t@sigill.intra.peff.net","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2016-10-11T19:07:40Z","receivedAt":"2016-10-11T19:09:08Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Tue, Oct 11, 2016 at 12:01 PM, Jeff King <peff@peff.net> wrote:\n>\n> My implementation is a little more complicated because it's also setting\n> things up for grouping by trailers (so you can group by \"signed-off-by\",\n> for example). I don't know if that's useful to your or not.\n\nHmm. Maybe in theory. But probably not in reality - it's just not\nunique enough (ie there are generally multiple, and if you choose the\nfirst/last, it should be the same as author/committer, so it doesn't\nactually add anything).\n\nThere are possibly other things that *could* be grouped by and might be useful:\n\n - main subdirectory it touches (I've often wanted that)\n\n - rough size of diff or number of files it touches\n\nbut realistically both are painful enough that it probably doesn't\nmake sense to do in some low-level helper.\n\n> I'm fine with this less invasive version, but a few suggestions:\n>\n>  - do you want to call it --group-by=committer (with --group-by=author\n>    as the default), which could later extend naturally to other forms of\n>    grouping?\n\nHonestly, it's probably the more generic one, but especially for\none-off commands that aren't that common, it's a pain to write. When\ntesting it, I literally just used \"-c\" for that reason.\n\nI wrote the patch because I've wanted this before, but it's a \"once or\ntwice a merge window\" thing for me, so ..\n\n>  - you might want to steal the tests and documentation from my patch\n>    (though obviously they would need tweaked to match your interface)\n\nHeh. Yes.\n\n          Linus\n"},{"id":"303910","messageId":"20161011191712.ms3n5uzufko7c7z2@sigill.intra.peff.net","threadId":"44277","inReplyTo":"CA+55aFzw24pHGOYFBFVvTbU1Cudcr8zcPt_RvdQSxrKY5weCbQ@mail.gmail.com","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-11T19:17:13Z","receivedAt":"2016-10-11T19:17:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 11, 2016 at 12:07:40PM -0700, Linus Torvalds wrote:\n\n> On Tue, Oct 11, 2016 at 12:01 PM, Jeff King <peff@peff.net> wrote:\n> >\n> > My implementation is a little more complicated because it's also setting\n> > things up for grouping by trailers (so you can group by \"signed-off-by\",\n> > for example). I don't know if that's useful to your or not.\n> \n> Hmm. Maybe in theory. But probably not in reality - it's just not\n> unique enough (ie there are generally multiple, and if you choose the\n> first/last, it should be the same as author/committer, so it doesn't\n> actually add anything).\n\nThe implementation I did credited each commit multiple times if the\ntrailer appeared more than once. If you want to play with it, you can\nfetch it from:\n\n  git://github.com/peff jk/shortlog-ident\n\nand then something like:\n\n  git shortlog --ident=reviewed-by --format='...reviewed %an'\n\nworks. I haven't found it to really be useful for more than toy\nstatistic gathering, though.\n\n> There are possibly other things that *could* be grouped by and might be useful:\n> \n>  - main subdirectory it touches (I've often wanted that)\n> \n>  - rough size of diff or number of files it touches\n> \n> but realistically both are painful enough that it probably doesn't\n> make sense to do in some low-level helper.\n\nYeah, I think there's a lot of policy there in what counts as \"main\",\nthe rough sizes, etc. I've definitely done queries like that before, but\nusually by piping \"log --numstat\" into perl. It's a minor pain to get\nthe data into perl data structures, but once you have it, you have a lot\nmore flexibility in what you can compute.\n\nThat might be aided by providing more structured machine-readable output\nfrom git, like JSON (which I don't particularly like, but it's kind-of a\nstandard, and it sure as hell beats XML). But obviously that's another\ntopic entirely.\n\n> > I'm fine with this less invasive version, but a few suggestions:\n> >\n> >  - do you want to call it --group-by=committer (with --group-by=author\n> >    as the default), which could later extend naturally to other forms of\n> >    grouping?\n> \n> Honestly, it's probably the more generic one, but especially for\n> one-off commands that aren't that common, it's a pain to write. When\n> testing it, I literally just used \"-c\" for that reason.\n\nIt's not the end of the world to call it \"-c\" now, and later define \"-c\"\nas a shorthand for \"--group-by=committer\", if and when the latter comes\ninto existence.\n\nKeep in mind that shortlog takes arbitrary revision options, too, and\n\"-c\" is defined there for combined diffs. I can't think of a good reason\nto want to pass it to shortlog, though, so I don't think it's a big\nloss.\n\n-Peff\n"},{"id":"307865","messageId":"CA+55aFxSQ2wxU3cA+8uqS-W8mbobF35dVCZow2BcixGOOvGVFQ@mail.gmail.com","threadId":"44277","inReplyTo":"CA+55aFzWkE43rSm-TJNKkHq4F3eOiGR0-Bo9V1=a1s=vQ0KPqQ@mail.gmail.com","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2016-12-15T21:29:47Z","receivedAt":"2016-12-15T21:29:54Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"Just a ping on this patch..\n\nOn Tue, Oct 11, 2016 at 11:45 AM, Linus Torvalds\n<torvalds@linux-foundation.org> wrote:\n> In some situations you may want to group the commits not by author,\n> but by committer instead.\n>\n> For example, when I just wanted to look up what I'm still missing from\n> linux-next in the current merge window [..]\n\nIt's another merge window later for the kernel, and I just re-applied\nthis patch to my git tree because I still want to know teh committer\ninformation rather than the authorship information, and it still seems\nto be the simplest way to do that.\n\nJeff had apparently done something similar as part of a bigger\npatch-series, but I don't see that either. I really don't care very\nmuch how this is done, but I do find this very useful, I do things\nlike\n\n   git shortlog -cnse linus..next |\n        head -20 |\n        cut -f2 |\n        sed 's/$/,/'\n\nto generate a nice list of the top-20 committers that I haven't gotten\npull requests from yet.\n\nYes, I can just maintain this myself, and maybe nobody else needs it,\nbut it's pretty simple and straightforward, and there didn't seem to\nbe any real reason not to have the option..\n\n                 Linus\n"},{"id":"307871","messageId":"xmqqoa0cu3nn.fsf@gitster.mtv.corp.google.com","threadId":"44277","inReplyTo":"CA+55aFxSQ2wxU3cA+8uqS-W8mbobF35dVCZow2BcixGOOvGVFQ@mail.gmail.com","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-16T00:19:08Z","receivedAt":"2016-12-16T00:20:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> Just a ping on this patch..\n> \n> Jeff had apparently done something similar as part of a bigger\n> patch-series, but I don't see that either. I really don't care very\n> much how this is done, but I do find this very useful, ...\n>\n> Yes, I can just maintain this myself, and maybe nobody else needs it,\n> but it's pretty simple and straightforward, and there didn't seem to\n> be any real reason not to have the option..\n\nThis fell off the radar partly because of the distractions like\n\"there are other attempts and other ways\", and also because the\nmessage was not a text-plain that can be reviewed inline.  Let me\ntry to dig it up from the mail archive to see if I can find it.\n"},{"id":"307873","messageId":"CA+55aFySBc1Nd_xYZmXF9tdynjW+udsEz+PtkQpkrPjeFVcfDw@mail.gmail.com","threadId":"44277","inReplyTo":"xmqqoa0cu3nn.fsf@gitster.mtv.corp.google.com","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2016-12-16T01:39:53Z","receivedAt":"2016-12-16T01:39:59Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Thu, Dec 15, 2016 at 4:19 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> This fell off the radar partly because of the distractions like\n> \"there are other attempts and other ways\", and also because the\n> message was not a text-plain that can be reviewed inline.  Let me\n> try to dig it up from the mail archive to see if I can find it.\n\nSorry, I'll just re-send it without the attachment. I prefer inline\nmyself, but I thought you didn't care (and gmail makes it\nunnecessarily hard).\n\n                Linus\n"},{"id":"307874","messageId":"alpine.LFD.2.20.1612151741280.3583@i7","threadId":"44277","inReplyTo":"xmqqoa0cu3nn.fsf@gitster.mtv.corp.google.com","subject":"[PATCH 1/1] Allow \"git shortlog\" to group by committer information","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2016-12-16T01:45:14Z","receivedAt":"2016-12-16T01:45:21Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nFrom: Linus Torvalds <torvalds@linux-foundation.org>\nSubject: Allow \"git shortlog\" to group by committer information\nDate: Tue, 11 Oct 2016 11:45:58 -0700\n\nIn some situations you may want to group the commits not by author, but by \ncommitter instead.\n\nFor example, when I just wanted to look up what I'm still missing from \nlinux-next in the current merge window, I don't care so much about who \nwrote a patch, as what git tree it came from, which generally boils down \nto \"who committed it\".\n\nSo make git shortlog take a \"-c\" or \"--committer\" option to switch \ngrouping to that. During the merge window this allows me to do things like\n\n   git shortlog -cnse linus..next |\n        head -20 |\n        cut -f2 |\n        sed 's/$/,/'\n\nto easily create a list of the top-20 committers that I haven't gotten \npull requests from yet (the committer is not necessarily the person who \nwill send the pull request, but it's a reasonably good approximation).\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n builtin/shortlog.c | 15 ++++++++++++---\n shortlog.h         |  1 +\n 2 files changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex ba0e1154a..c9585d475 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -117,11 +117,15 @@ static void read_from_stdin(struct shortlog *log)\n {\n \tstruct strbuf author = STRBUF_INIT;\n \tstruct strbuf oneline = STRBUF_INIT;\n+\tstatic const char *author_match[2] = { \"Author: \", \"author \" };\n+\tstatic const char *committer_match[2] = { \"Commit: \", \"committer \" };\n+\tconst char **match;\n \n+\tmatch = log->committer ? committer_match : author_match;\n \twhile (strbuf_getline_lf(&author, stdin) != EOF) {\n \t\tconst char *v;\n-\t\tif (!skip_prefix(author.buf, \"Author: \", &v) &&\n-\t\t    !skip_prefix(author.buf, \"author \", &v))\n+\t\tif (!skip_prefix(author.buf, match[0], &v) &&\n+\t\t    !skip_prefix(author.buf, match[1], &v))\n \t\t\tcontinue;\n \t\twhile (strbuf_getline_lf(&oneline, stdin) != EOF &&\n \t\t       oneline.len)\n@@ -140,6 +144,7 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \tstruct strbuf author = STRBUF_INIT;\n \tstruct strbuf oneline = STRBUF_INIT;\n \tstruct pretty_print_context ctx = {0};\n+\tconst char *fmt;\n \n \tctx.fmt = CMIT_FMT_USERFORMAT;\n \tctx.abbrev = log->abbrev;\n@@ -148,7 +153,9 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)\n \tctx.date_mode.type = DATE_NORMAL;\n \tctx.output_encoding = get_log_output_encoding();\n \n-\tformat_commit_message(commit, \"%an <%ae>\", &author, &ctx);\n+\tfmt = log->committer ? \"%cn <%ce>\" : \"%an <%ae>\";\n+\n+\tformat_commit_message(commit, fmt, &author, &ctx);\n \tif (!log->summary) {\n \t\tif (log->user_format)\n \t\t\tpretty_print_commit(&ctx, commit, &oneline);\n@@ -238,6 +245,8 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)\n \tint nongit = !startup_info->have_repository;\n \n \tconst struct option options[] = {\n+\t\tOPT_BOOL('c', \"committer\", &log.committer,\n+\t\t\t N_(\"Group by committer rather than author\")),\n \t\tOPT_BOOL('n', \"numbered\", &log.sort_by_number,\n \t\t\t N_(\"sort output according to the number of commits per author\")),\n \t\tOPT_BOOL('s', \"summary\", &log.summary,\ndiff --git a/shortlog.h b/shortlog.h\nindex 5a326c686..5d64cfe92 100644\n--- a/shortlog.h\n+++ b/shortlog.h\n@@ -13,6 +13,7 @@ struct shortlog {\n \tint in2;\n \tint user_format;\n \tint abbrev;\n+\tint committer;\n \n \tchar *common_repo_prefix;\n \tint email;\n"},{"id":"307875","messageId":"CA+55aFxwDQAqXb+QYzS+3uZRQCFwHMofZXeWNns9t9Z7uNV4Wg@mail.gmail.com","threadId":"44277","inReplyTo":"3720429.U3o1zloj4W@thunderbird","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2016-12-16T02:00:01Z","receivedAt":"2016-12-16T02:00:52Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Thu, Dec 15, 2016 at 5:51 PM, Stephen & Linda Smith <ischis2@cox.net> wrote:\n>\n> Why does gmail make it unnecessarily hard?\n\nI read email with the gmail web interface, which is wonderful because\nof the server-side searching etc. The only real downside is the weak\nthreading, but you get used to it.\n\nI personally find IMAP and POP to be a tool of the devil, and have\nnever had a good experience with them as a mail interface. In theory\nIMAP is supposed to support server-side searches, in practice it never\nworked for me.\n\nBut the problem with sending patches using the web interface is that\nyou cannot attach things inline without gmail screwing up whitespace.\n\nI suggested to some googler that a \"attach inline\" checkbox in the\nwould be a wonderful option for text attachments, but considering that\nthe android gmail app still has no text-only option I don't think that\nsuggestion went anywhere.\n\n> I thought that a good percentage of the kernel maintainers use git send-email.\n> what would make that command easier to use with gmail?\n\nOh, I can send inline stuff (as I just re-sent that patch), but then I\nhave to fire up alpine and do it the old-fashioned way. So it's an\nextra step. So since I spend all my time at the gmail web interface\n_anyway_, the attachment model ends up being the slightly more\nconvenient one.\n\nAnd sure, I could use git-send-email as that extra step instead, but\nI'd rather just use alpine. That's the extra step I do for some other\nthings (ie the \"200-email patch-bomb from Andrew Morton\" things - I'm\nnot using the web interface for _that_).\n\n                 Linus\n"},{"id":"307876","messageId":"3720429.U3o1zloj4W@thunderbird","threadId":"44277","inReplyTo":"xmqqoa0cu3nn.fsf@gitster.mtv.corp.google.com","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Stephen & Linda Smith","fromEmail":"ischis2@cox.net","sentAt":"2016-12-16T01:51:02Z","receivedAt":"2016-12-16T02:01:25Z","isPatch":false,"sender":{"key":"ishchis2@gmail.com","avatar":null},"body":"On Thursday, December 15, 2016 5:39:53 PM MST Linus Torvalds wrote:\n> On Thu, Dec 15, 2016 at 4:19 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> \n> Sorry, I'll just re-send it without the attachment. I prefer inline\n> myself, but I thought you didn't care (and gmail makes it\n> unnecessarily hard).\n> \n>                 Linus\n\nWhy does gmail make it unnecessarily hard?  \n\nI thought that a good percentage of the kernel maintainers use git send-email.   \nwhat would make that command easier to use with gmail?\n\nsps\n\n"},{"id":"307882","messageId":"xmqqy3zgtqsz.fsf@gitster.mtv.corp.google.com","threadId":"44277","inReplyTo":"CA+55aFySBc1Nd_xYZmXF9tdynjW+udsEz+PtkQpkrPjeFVcfDw@mail.gmail.com","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-16T04:56:44Z","receivedAt":"2016-12-16T04:57:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Thu, Dec 15, 2016 at 4:19 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> This fell off the radar partly because of the distractions like\n>> \"there are other attempts and other ways\", and also because the\n>> message was not a text-plain that can be reviewed inline.  Let me\n>> try to dig it up from the mail archive to see if I can find it.\n>\n> Sorry, I'll just re-send it without the attachment. I prefer inline\n> myself, but I thought you didn't care (and gmail makes it\n> unnecessarily hard).\n\nThanks.  \n\nI do care, but I try to be lenient for inexperienced contriburors,\nwhich you don't qualify ;-) Experienced ones are held to a higher\nstandard.\n\n"},{"id":"307885","messageId":"20161216133940.hu474phggdslh6ka@sigill.intra.peff.net","threadId":"44277","inReplyTo":"CA+55aFxSQ2wxU3cA+8uqS-W8mbobF35dVCZow2BcixGOOvGVFQ@mail.gmail.com","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-16T13:39:40Z","receivedAt":"2016-12-16T13:39:50Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 15, 2016 at 01:29:47PM -0800, Linus Torvalds wrote:\n\n> On Tue, Oct 11, 2016 at 11:45 AM, Linus Torvalds\n> <torvalds@linux-foundation.org> wrote:\n> > In some situations you may want to group the commits not by author,\n> > but by committer instead.\n> >\n> > For example, when I just wanted to look up what I'm still missing from\n> > linux-next in the current merge window [..]\n> \n> It's another merge window later for the kernel, and I just re-applied\n> this patch to my git tree because I still want to know teh committer\n> information rather than the authorship information, and it still seems\n> to be the simplest way to do that.\n> \n> Jeff had apparently done something similar as part of a bigger\n> patch-series, but I don't see that either. I really don't care very\n> much how this is done, but I do find this very useful, I do things\n> like\n\nSorry if I de-railed the earlier conversation. The shortlog\ngroup-by-trailer work didn't seem useful enough for me to make it a\npriority.\n\nI'm OK with the approach your patch takes, but I think there were some\nunresolved issues:\n\n  - are we OK taking the short \"-c\" for this, or do we want\n    \"--group-by=committer\" or something like it?\n\n  - no tests; you can steal the general form from my [1]\n\n  - no documentation (can also be stolen from [1], though the syntax is\n    quite different)\n\n-Peff\n\n[1] http://public-inbox.org/git/20151229073515.GK8842@sigill.intra.peff.net/\n"},{"id":"307886","messageId":"20161216135141.yhas67pzfm7bxxum@sigill.intra.peff.net","threadId":"44277","inReplyTo":"20161216133940.hu474phggdslh6ka@sigill.intra.peff.net","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-16T13:51:41Z","receivedAt":"2016-12-16T13:51:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 16, 2016 at 08:39:40AM -0500, Jeff King wrote:\n\n> I'm OK with the approach your patch takes, but I think there were some\n> unresolved issues:\n> \n>   - are we OK taking the short \"-c\" for this, or do we want\n>     \"--group-by=committer\" or something like it?\n> \n>   - no tests; you can steal the general form from my [1]\n> \n>   - no documentation (can also be stolen from [1], though the syntax is\n>     quite different)\n\nBeing moved by the holiday spirit, I wrote a patch to address the latter\ntwo. ;)\n\nIt obviously would need updating if we switch away from \"-c\", but I\nthink I am OK with the short \"-c\" (even if we add a more exotic grouping\noption later, this can remain as a short synonym).\n\n-- >8 --\nSubject: [PATCH] shortlog: test and document --committer option\n\nThis puts the final touches on the feature added by\nfbfda15fb8 (shortlog: group by committer information,\n2016-10-11).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/git-shortlog.txt |  4 ++++\n t/t4201-shortlog.sh            | 13 +++++++++++++\n 2 files changed, 17 insertions(+)\n\ndiff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\nindex 31af7f2736..ee6c5476c1 100644\n--- a/Documentation/git-shortlog.txt\n+++ b/Documentation/git-shortlog.txt\n@@ -47,6 +47,10 @@ OPTIONS\n \n \tEach pretty-printed commit will be rewrapped before it is shown.\n \n+-c::\n+--committer::\n+\tCollect and show committer identities instead of authors.\n+\n -w[<width>[,<indent1>[,<indent2>]]]::\n \tLinewrap the output by wrapping each line at `width`.  The first\n \tline of each entry is indented by `indent1` spaces, and the second\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex ae08b57712..6c7c637481 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -190,4 +190,17 @@ test_expect_success 'shortlog with --output=<file>' '\n \ttest_line_count = 3 shortlog\n '\n \n+test_expect_success 'shortlog --committer (internal)' '\n+\tcat >expect <<-\\EOF &&\n+\t     3\tC O Mitter\n+\tEOF\n+\tgit shortlog -nsc HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'shortlog --committer (external)' '\n+\tgit log --format=full | git shortlog -nsc >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.11.0.348.g960a0b554\n\n"},{"id":"307890","messageId":"xmqqlgvfu6ll.fsf@gitster.mtv.corp.google.com","threadId":"44277","inReplyTo":"20161216135141.yhas67pzfm7bxxum@sigill.intra.peff.net","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-16T17:27:50Z","receivedAt":"2016-12-16T17:27:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> It obviously would need updating if we switch away from \"-c\", but I\n> think I am OK with the short \"-c\" (even if we add a more exotic grouping\n> option later, this can remain as a short synonym).\n\nYeah, I think it probably is OK.  \n\nAs it is very clear that \"group by author\" is the default, there is\nno need to add the corresponding \"-a/--author\" option, either.  The\nfact that \"--no-committer\" can countermand an earlier \"--committer\"\non the command line is just how options work, so it probably does\nnot deserve a separate mention, either.\n\nThanks.\n\n> -- >8 --\n> Subject: [PATCH] shortlog: test and document --committer option\n>\n> This puts the final touches on the feature added by\n> fbfda15fb8 (shortlog: group by committer information,\n> 2016-10-11).\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  Documentation/git-shortlog.txt |  4 ++++\n>  t/t4201-shortlog.sh            | 13 +++++++++++++\n>  2 files changed, 17 insertions(+)\n>\n> diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt\n> index 31af7f2736..ee6c5476c1 100644\n> --- a/Documentation/git-shortlog.txt\n> +++ b/Documentation/git-shortlog.txt\n> @@ -47,6 +47,10 @@ OPTIONS\n>  \n>  \tEach pretty-printed commit will be rewrapped before it is shown.\n>  \n> +-c::\n> +--committer::\n> +\tCollect and show committer identities instead of authors.\n> +\n>  -w[<width>[,<indent1>[,<indent2>]]]::\n>  \tLinewrap the output by wrapping each line at `width`.  The first\n>  \tline of each entry is indented by `indent1` spaces, and the second\n> diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\n> index ae08b57712..6c7c637481 100755\n> --- a/t/t4201-shortlog.sh\n> +++ b/t/t4201-shortlog.sh\n> @@ -190,4 +190,17 @@ test_expect_success 'shortlog with --output=<file>' '\n>  \ttest_line_count = 3 shortlog\n>  '\n>  \n> +test_expect_success 'shortlog --committer (internal)' '\n> +\tcat >expect <<-\\EOF &&\n> +\t     3\tC O Mitter\n> +\tEOF\n> +\tgit shortlog -nsc HEAD >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'shortlog --committer (external)' '\n> +\tgit log --format=full | git shortlog -nsc >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>  test_done\n"},{"id":"308155","messageId":"16b115e0-3a7e-a5c2-1526-44bbcfc97db8@kdbg.org","threadId":"44277","inReplyTo":"20161216135141.yhas67pzfm7bxxum@sigill.intra.peff.net","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-12-20T18:12:32Z","receivedAt":"2016-12-20T18:12:45Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 16.12.2016 um 14:51 schrieb Jeff King:\n> diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\n> index ae08b57712..6c7c637481 100755\n> --- a/t/t4201-shortlog.sh\n> +++ b/t/t4201-shortlog.sh\n> @@ -190,4 +190,17 @@ test_expect_success 'shortlog with --output=<file>' '\n>  \ttest_line_count = 3 shortlog\n>  '\n>\n> +test_expect_success 'shortlog --committer (internal)' '\n> +\tcat >expect <<-\\EOF &&\n> +\t     3\tC O Mitter\n> +\tEOF\n> +\tgit shortlog -nsc HEAD >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'shortlog --committer (external)' '\n> +\tgit log --format=full | git shortlog -nsc >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>  test_done\n>\n\nMay I kindly ask you to make this work on Windows, too? Just\n\nsed -i -e s/MINGW/MINGW,HAVENOT/ t4201-shortlog.sh\n\non your Linux box and make it pass the tests.\n\nThank you so much in advance.\n\n-- Hannes\n\n"},{"id":"308156","messageId":"xmqq60melazp.fsf@gitster.mtv.corp.google.com","threadId":"44277","inReplyTo":"16b115e0-3a7e-a5c2-1526-44bbcfc97db8@kdbg.org","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-20T18:19:06Z","receivedAt":"2016-12-20T18:19:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Am 16.12.2016 um 14:51 schrieb Jeff King:\n>> diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\n>> index ae08b57712..6c7c637481 100755\n>> --- a/t/t4201-shortlog.sh\n>> +++ b/t/t4201-shortlog.sh\n>> @@ -190,4 +190,17 @@ test_expect_success 'shortlog with --output=<file>' '\n>>  \ttest_line_count = 3 shortlog\n>>  '\n>>\n>> +test_expect_success 'shortlog --committer (internal)' '\n>> +\tcat >expect <<-\\EOF &&\n>> +\t     3\tC O Mitter\n>> +\tEOF\n>> +\tgit shortlog -nsc HEAD >actual &&\n>> +\ttest_cmp expect actual\n>> +'\n>> +\n>> +test_expect_success 'shortlog --committer (external)' '\n>> +\tgit log --format=full | git shortlog -nsc >actual &&\n>> +\ttest_cmp expect actual\n>> +'\n>> +\n>>  test_done\n>>\n>\n> May I kindly ask you to make this work on Windows, too? Just\n>\n> sed -i -e s/MINGW/MINGW,HAVENOT/ t4201-shortlog.sh\n\nHAVENOT???\n\n>\n> on your Linux box and make it pass the tests.\n>\n> Thank you so much in advance.\n>\n> -- Hannes\n"},{"id":"308157","messageId":"xmqq1sx2lara.fsf@gitster.mtv.corp.google.com","threadId":"44277","inReplyTo":"xmqq60melazp.fsf@gitster.mtv.corp.google.com","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-20T18:24:09Z","receivedAt":"2016-12-20T18:24:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> May I kindly ask you to make this work on Windows, too? Just\n>>\n>> sed -i -e s/MINGW/MINGW,HAVENOT/ t4201-shortlog.sh\n>\n> HAVENOT???\n>>\n>> on your Linux box and make it pass the tests.\n>>\n>> Thank you so much in advance.\n\nAh, I think I am slower than my usual today.\n"},{"id":"308159","messageId":"xmqqvauejvnr.fsf@gitster.mtv.corp.google.com","threadId":"44277","inReplyTo":"xmqq1sx2lara.fsf@gitster.mtv.corp.google.com","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-20T18:35:36Z","receivedAt":"2016-12-20T18:35:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>>> May I kindly ask you to make this work on Windows, too? Just\n>>>\n>>> sed -i -e s/MINGW/MINGW,HAVENOT/ t4201-shortlog.sh\n>>\n>> HAVENOT???\n>>>\n>>> on your Linux box and make it pass the tests.\n>>>\n>>> Thank you so much in advance.\n>\n> Ah, I think I am slower than my usual today.\n\n-- >8 --\nSubject: SQUASH???\n\nMake sure the test does not depend on the result of the previous\ntests; with MINGW prerequisite satisfied, a \"reset to original and\nrebuild\" in an earlier test was skipped, resulting in different\nhistory being tested with this and the next tests.\n\n---\n t/t4201-shortlog.sh | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh\nindex 6c7c637481..9df054bf05 100755\n--- a/t/t4201-shortlog.sh\n+++ b/t/t4201-shortlog.sh\n@@ -191,8 +191,14 @@ test_expect_success 'shortlog with --output=<file>' '\n '\n \n test_expect_success 'shortlog --committer (internal)' '\n+\tgit checkout --orphan side &&\n+\tgit commit --allow-empty -m one &&\n+\tgit commit --allow-empty -m two &&\n+\tGIT_COMMITTER_NAME=\"Sin Nombre\" git commit --allow-empty -m three &&\n+\n \tcat >expect <<-\\EOF &&\n-\t     3\tC O Mitter\n+\t     2\tC O Mitter\n+\t     1\tSin Nombre\n \tEOF\n \tgit shortlog -nsc HEAD >actual &&\n \ttest_cmp expect actual\n"},{"id":"308160","messageId":"d2ac90d6-c4f4-a759-a6e2-2d7fe5bb1c1d@kdbg.org","threadId":"44277","inReplyTo":"xmqqvauejvnr.fsf@gitster.mtv.corp.google.com","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-12-20T18:52:21Z","receivedAt":"2016-12-20T18:52:28Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 20.12.2016 um 19:35 schrieb Junio C Hamano:\n>  test_expect_success 'shortlog --committer (internal)' '\n> +\tgit checkout --orphan side &&\n> +\tgit commit --allow-empty -m one &&\n> +\tgit commit --allow-empty -m two &&\n> +\tGIT_COMMITTER_NAME=\"Sin Nombre\" git commit --allow-empty -m three &&\n\nClever! Thank you. Will test in 12 hours.\n\n> +\n>  \tcat >expect <<-\\EOF &&\n> -\t     3\tC O Mitter\n> +\t     2\tC O Mitter\n> +\t     1\tSin Nombre\n>  \tEOF\n>  \tgit shortlog -nsc HEAD >actual &&\n>  \ttest_cmp expect actual\n>\n\n"},{"id":"308191","messageId":"20161221032221.s7jmgnfrr6tyuyuk@sigill.intra.peff.net","threadId":"44277","inReplyTo":"xmqqvauejvnr.fsf@gitster.mtv.corp.google.com","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-21T03:22:21Z","receivedAt":"2016-12-21T03:22:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 20, 2016 at 10:35:36AM -0800, Junio C Hamano wrote:\n\n> -- >8 --\n> Subject: SQUASH???\n> \n> Make sure the test does not depend on the result of the previous\n> tests; with MINGW prerequisite satisfied, a \"reset to original and\n> rebuild\" in an earlier test was skipped, resulting in different\n> history being tested with this and the next tests.\n\nYeah, this looks good, and obviously correct.\n\nI do wonder if in general it should be the responsibility of skippable\ntests to make sure we end up with the same state whether they are run or\nnot. That might manage the complexity more. But I certainly don't mind\ntests being defensive like you have here.\n\n-Peff\n"},{"id":"308193","messageId":"CA+P7+xrMgzFcuqwBg6z2_ZPgAVKwLX2eyK6D4C0v-c3zAMFqUg@mail.gmail.com","threadId":"44277","inReplyTo":"20161221032221.s7jmgnfrr6tyuyuk@sigill.intra.peff.net","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-12-21T07:55:45Z","receivedAt":"2016-12-21T07:56:11Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Tue, Dec 20, 2016 at 7:22 PM, Jeff King <peff@peff.net> wrote:\n> On Tue, Dec 20, 2016 at 10:35:36AM -0800, Junio C Hamano wrote:\n>\n>> -- >8 --\n>> Subject: SQUASH???\n>>\n>> Make sure the test does not depend on the result of the previous\n>> tests; with MINGW prerequisite satisfied, a \"reset to original and\n>> rebuild\" in an earlier test was skipped, resulting in different\n>> history being tested with this and the next tests.\n>\n> Yeah, this looks good, and obviously correct.\n>\n> I do wonder if in general it should be the responsibility of skippable\n> tests to make sure we end up with the same state whether they are run or\n> not. That might manage the complexity more. But I certainly don't mind\n> tests being defensive like you have here.\n>\n> -Peff\n\nThat seems like a good idea, but I'm not sure how you would implement\nit in practice? Would we just \"rely\" on a skipable test having a \"do\nthis if we skip, instead\" block? That would be easier to spot but I\nthink still relies on the skip-able tests being careful?\n\nThanks,\nJake\n"},{"id":"308199","messageId":"20161221160412.54d5ozyoxetwl3oy@sigill.intra.peff.net","threadId":"44277","inReplyTo":"CA+P7+xrMgzFcuqwBg6z2_ZPgAVKwLX2eyK6D4C0v-c3zAMFqUg@mail.gmail.com","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-21T16:04:13Z","receivedAt":"2016-12-21T16:04:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 20, 2016 at 11:55:45PM -0800, Jacob Keller wrote:\n\n> > I do wonder if in general it should be the responsibility of skippable\n> > tests to make sure we end up with the same state whether they are run or\n> > not. That might manage the complexity more. But I certainly don't mind\n> > tests being defensive like you have here.\n> >\n> > -Peff\n> \n> That seems like a good idea, but I'm not sure how you would implement\n> it in practice? Would we just \"rely\" on a skipable test having a \"do\n> this if we skip, instead\" block? That would be easier to spot but I\n> think still relies on the skip-able tests being careful?\n\nYes, it definitely means the skip-able tests would have to be careful.\nBut it's putting the onus on them, rather than on all the other tests.\n\nIf the rule is \"the on-disk state must be the same whether or not the\ntest runs\", then I suspect many tests could get by with a\ntest_when_finished, like:\n\n  test_expect_success FOO 'test --foo knob' '\n\tgit commit -m \"new commit for test\" &&\n\ttest_when_finished \"git reset --hard HEAD^\" &&\n\tgit log --foo >actual &&\n\ttest_cmp expect actual\n  '\n\nIt gets harder if you have multiple such tests that rely on intermediate\nstate. Probably you'd have:\n\n  test_expect_success FOO 'clean up FOO state' '\n\tgit reset --hard HEAD^\n  '\n\nat the end of the sequence.\n\nI dunno. It is unclear to me whether such a rule is worth it.\nPreemptively fighting these state-based bugs before they occur is a nice\nthought, but I think it may end up being more work to write the tests\nthat way than it is to simply find and fix the bugs when they occur. Of\ncourse it also changes where the work falls (the test writers versus the\nbug hunters, and given that !MINGW is our most common prereq, I think\nWindows devs are over-represented in the latter case).\n\n-Peff\n"},{"id":"308213","messageId":"xmqqh95xhv12.fsf@gitster.mtv.corp.google.com","threadId":"44277","inReplyTo":"20161221032221.s7jmgnfrr6tyuyuk@sigill.intra.peff.net","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-21T20:44:25Z","receivedAt":"2016-12-21T20:44:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I do wonder if in general it should be the responsibility of skippable\n> tests to make sure we end up with the same state whether they are run or\n> not. That might manage the complexity more. But I certainly don't mind\n> tests being defensive like you have here.\n\nIf we speak \"in general\", I would say that any test should be\nprepared to be turned into a skippable one, and they should all make\nsure they leave the same state whether they are skipped, they\nsucceed, or they fail in the middle.\n\nThat can theoretically be achievable (e.g. you assume you would\nalways start from an empty repository, do your thing and arrange to\nleave an empty repository by doing test_when_finished), and the\ncognitive cost of developers to do so can be reduced by teaching\ntest_expect_{success/failure} helpers to be responsible for the\n\"arrange to leave an empty repository\" part.  But it is quite a big\ndeparture from the way our tests are currently done, i.e. prepare\nthe environment once and then each of multiple tests observes one\nthing in that environment (e.g. \"does it work well with --dry-run?\nhow about without?\").\n\nAlso it will make the runtime cost of the tests a lot larger, as\nsetup and teardown need to happen for each individual test.  So I do\nnot think it is a good goal in practice.\n\nPerhaps what you suggest may be a good middle-ground.  When you add\nprerequisite to an existing test, it will become your responsibility\nto make sure the test will leave the same state.  That way, you\nwould know that tests that come later will not be affected by your\nchange.\n\n"},{"id":"308217","messageId":"a74c5915-d00e-cf85-1fbe-94647586c8aa@kdbg.org","threadId":"44277","inReplyTo":"d2ac90d6-c4f4-a759-a6e2-2d7fe5bb1c1d@kdbg.org","subject":"Re: Allow \"git shortlog\" to group by committer information","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-12-21T21:09:48Z","receivedAt":"2016-12-21T21:10:02Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 20.12.2016 um 19:52 schrieb Johannes Sixt:\n> Am 20.12.2016 um 19:35 schrieb Junio C Hamano:\n>>  test_expect_success 'shortlog --committer (internal)' '\n>> +    git checkout --orphan side &&\n>> +    git commit --allow-empty -m one &&\n>> +    git commit --allow-empty -m two &&\n>> +    GIT_COMMITTER_NAME=\"Sin Nombre\" git commit --allow-empty -m three &&\n>\n> Clever! Thank you. Will test in 12 hours.\n>\n>> +\n>>      cat >expect <<-\\EOF &&\n>> -         3    C O Mitter\n>> +         2    C O Mitter\n>> +         1    Sin Nombre\n>>      EOF\n>>      git shortlog -nsc HEAD >actual &&\n>>      test_cmp expect actual\n>>\n>\n\nI confirm that t4201 now passes on Windows with this fixup.\n\n-- Hannes\n\n"}]}