{"thread":{"id":"39354","subject":"[PATCH v2] pull: handle --log=","startedAt":"2015-05-18T13:39:56Z","lastAt":"2015-05-22T13:29:55Z","messageCount":22,"participants":["Paul Tan","Johannes Schindelin","Junio C Hamano","Dennis Kaarsemaker","Stefan Beller"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"261439","messageId":"1431956396-21788-1-git-send-email-pyokagan@gmail.com","threadId":"39354","inReplyTo":null,"subject":"[PATCH v2] pull: handle --log=","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-18T13:39:56Z","receivedAt":"2015-05-18T13:39:56Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since efb779f (merge, pull: add '--(no-)log' command line option,\n2008-04-06) git-pull supported the (--no-)log switch and would pass it\nto git-merge.\n\n96e9420 (merge: Make '--log' an integer option for number of shortlog\nentries, 2010-09-08) implemented support for the --log= switch, which\nwould explicitly set the number of shortlog entries. However, git-pull\ndoes not recognize this option, and will instead pass it to git-fetch,\nleading to \"unknown option\" errors.\n\nFix this by matching --log=* in addition to --log and --no-log.\n\nImplement a test for this use case.\n\nSigned-off-by: Paul Tan \n---\nThis is a re-roll of [1].\n\nThe only change is the removal of \"verbose\".\n\nThanks Matthieu for the review last round.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/268961\n\n git-pull.sh | 4 ++--\n t/t5524-pull-msg.sh | 17 +++++++++++++++++\n 2 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 9ed01fd..5ff4545 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -81,8 +81,8 @@ do\n \t\tdiffstat=--no-stat ;;\n \t--stat|--summary)\n \t\tdiffstat=--stat ;;\n-\t--log|--no-log)\n-\t\tlog_arg=$1 ;;\n+\t--log|--log=*|--no-log)\n+\t\tlog_arg=\"$1\" ;;\n \t--no-c|--no-co|--no-com|--no-comm|--no-commi|--no-commit)\n \t\tno_commit=--no-commit ;;\n \t--c|--co|--com|--comm|--commi|--commit)\ndiff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh\nindex 8cccecc..eebb8c9 100755\n--- a/t/t5524-pull-msg.sh\n+++ b/t/t5524-pull-msg.sh\n@@ -17,6 +17,9 @@ test_expect_success setup '\n \t\tgit commit -m \"add bfile\"\n \t) &&\n \ttest_tick && test_tick &&\n+\techo \"second\" >afile &&\n+\tgit add afile &&\n+\tgit commit -m \"second commit\" &&\n \techo \"original $dollar\" >afile &&\n \tgit add afile &&\n \tgit commit -m \"do not clobber $dollar signs\"\n@@ -32,4 +35,18 @@ test_expect_success pull '\n )\n '\n \n+test_expect_failure '--log=1 limits shortlog length' '\n+(\n+\tcd cloned &&\n+\tgit reset --hard HEAD^ &&\n+\ttest \"$(cat afile)\" = original &&\n+\ttest \"$(cat bfile)\" = added &&\n+\tgit pull --log=1 &&\n+\tgit log -3 &&\n+\tgit cat-file commit HEAD >result &&\n+\tgrep Dollar result &&\n+\t! grep \"second commit\" result\n+)\n+'\n+\n test_done\n-- \n2.1.4\n"},{"id":"261450","messageId":"5661061272076a1883cfde1087be4a42@www.dscho.org","threadId":"39354","inReplyTo":"1431956396-21788-1-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-18T14:53:56Z","receivedAt":"2015-05-18T14:53:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn 2015-05-18 15:39, Paul Tan wrote:\n\n> diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh\n> index 8cccecc..eebb8c9 100755\n> --- a/t/t5524-pull-msg.sh\n> +++ b/t/t5524-pull-msg.sh\n> @@ -17,6 +17,9 @@ test_expect_success setup '\n> \t\tgit commit -m \"add bfile\"\n> \t) &&\n> \ttest_tick && test_tick &&\n> +\techo \"second\" >afile &&\n> +\tgit add afile &&\n> +\tgit commit -m \"second commit\" &&\n> \techo \"original $dollar\" >afile &&\n> \tgit add afile &&\n\nI completely forgot to send a mail last time where I wanted to mention the test_commit function. I *think* that above invocation is equivalent to\n\n test_commit \"second commit\" afile second\n\nwith the additional benefit that `test_tick` is called automatically. Maybe you want to use that more concise function call?\n\nCiao,\nDscho\n"},{"id":"261470","messageId":"CACRoPnQKge+3gCnO79yTZHxyPE=XgCONApWPF7Ue7DSxr0E7Og@mail.gmail.com","threadId":"39354","inReplyTo":"1431956396-21788-1-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-18T15:15:36Z","receivedAt":"2015-05-18T15:15:36Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Just a mental note to myself,\n\nOn Mon, May 18, 2015 at 9:39 PM, Paul Tan wrote:\n> diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh\n> index 8cccecc..eebb8c9 100755\n> --- a/t/t5524-pull-msg.sh\n> +++ b/t/t5524-pull-msg.sh\n> @@ -17,6 +17,9 @@ test_expect_success setup '\n> git commit -m \"add bfile\"\n> ) &&\n> test_tick && test_tick &&\n> + echo \"second\" >afile &&\n> + git add afile &&\n> + git commit -m \"second commit\" &&\n> echo \"original $dollar\" >afile &&\n> git add afile &&\n> git commit -m \"do not clobber $dollar signs\"\n> @@ -32,4 +35,18 @@ test_expect_success pull '\n> )\n> '\n>\n> +test_expect_failure '--log=1 limits shortlog length' '\n\nUgh ><\n\ns/test_expect_failure/test_expect_success/\n\nWill respond to the other reviews tomorrow (it's bedtime in SGT).\n\n> +(\n> + cd cloned &&\n> + git reset --hard HEAD^ &&\n> + test \"$(cat afile)\" = original &&\n> + test \"$(cat bfile)\" = added &&\n> + git pull --log=1 &&\n> + git log -3 &&\n> + git cat-file commit HEAD >result &&\n> + grep Dollar result &&\n> + ! grep \"second commit\" result\n> +)\n> +'\n> +\n> test_done\n> --\n> 2.1.4\n\nThanks,\nPaul\n"},{"id":"261473","messageId":"8db56697772d5d5a4a01268e47db2bfe@www.dscho.org","threadId":"39354","inReplyTo":"CACRoPnQKge+3gCnO79yTZHxyPE=XgCONApWPF7Ue7DSxr0E7Og@mail.gmail.com","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-18T15:26:18Z","receivedAt":"2015-05-18T15:26:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Paul,\n\nOn 2015-05-18 17:15, Paul Tan wrote:\n> Just a mental note to myself,\n> \n> On Mon, May 18, 2015 at 9:39 PM, Paul Tan wrote:\n>> diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh\n>> index 8cccecc..eebb8c9 100755\n>> --- a/t/t5524-pull-msg.sh\n>> +++ b/t/t5524-pull-msg.sh\n>> @@ -17,6 +17,9 @@ test_expect_success setup '\n>> git commit -m \"add bfile\"\n>> ) &&\n>> test_tick && test_tick &&\n>> + echo \"second\" >afile &&\n>> + git add afile &&\n>> + git commit -m \"second commit\" &&\n>> echo \"original $dollar\" >afile &&\n>> git add afile &&\n>> git commit -m \"do not clobber $dollar signs\"\n>> @@ -32,4 +35,18 @@ test_expect_success pull '\n>> )\n>> '\n>>\n>> +test_expect_failure '--log=1 limits shortlog length' '\n> \n> Ugh ><\n> \n> s/test_expect_failure/test_expect_success/\n\nHeh, I missed that one myself.\n\n> Will respond to the other reviews tomorrow (it's bedtime in SGT).\n\nThat was quite some stuff you sent to the list, indeed. Good work!\n\nCiao,\nDscho\n"},{"id":"261486","messageId":"xmqq617pda0r.fsf@gitster.dls.corp.google.com","threadId":"39354","inReplyTo":"5661061272076a1883cfde1087be4a42@www.dscho.org","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-18T18:18:44Z","receivedAt":"2015-05-18T18:18:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi Paul,\n>\n> On 2015-05-18 15:39, Paul Tan wrote:\n>\n>> diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh\n>> index 8cccecc..eebb8c9 100755\n>> --- a/t/t5524-pull-msg.sh\n>> +++ b/t/t5524-pull-msg.sh\n>> @@ -17,6 +17,9 @@ test_expect_success setup '\n>> \t\tgit commit -m \"add bfile\"\n>> \t) &&\n>> \ttest_tick && test_tick &&\n>> +\techo \"second\" >afile &&\n>> +\tgit add afile &&\n>> +\tgit commit -m \"second commit\" &&\n>> \techo \"original $dollar\" >afile &&\n>> \tgit add afile &&\n>\n> I completely forgot to send a mail last time where I wanted to mention\n> the test_commit function. I *think* that above invocation is\n> equivalent to\n>\n> test_commit \"second commit\" afile second\n>\n> with the additional benefit that `test_tick` is called\n> automatically. Maybe you want to use that more concise function call?\n\nSounds like a good idea especially when you are testing commands\nthat does not do things to tags implicitly.\n\nI have been in love-hate relation with test_commit for a long time.\nIt is very nice that it gives us ticks (i.e. reproducible object\nnames) automatically and reduces the chance for new tests to forget\nto do so, but I found it extremely annoying that it wants to always\nadd a light-weight tag to every commit it creates.\n\nI suspect that this design decision largely comes from the fact that\nback then we did not exactly trust the syntax like \"master~3\" and\nwanted to have a set of fixed points, but often these auto-generated\ntags get in the way by either contaminating the refs namespaces (and\nwe are testing pull here, which worries me that test authors need to\nbe aware that these tags may be transferred to the destination) or\nanchoring the objects in the object database (e.g. when writing\n\"reset --hard branch branch~3 && prune\" tests, you need to remember\nto kill these tags).\n\nSo I dunno. I really wish test_commit didn't create tags and either\nleft the tagging to the calling script.\n"},{"id":"261548","messageId":"6b905c01c9b57abc05fb49117c28c10e@www.dscho.org","threadId":"39354","inReplyTo":"xmqq617pda0r.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-19T13:35:05Z","receivedAt":"2015-05-19T13:35:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn 2015-05-18 20:18, Junio C Hamano wrote:\n\n> I have been in love-hate relation with test_commit for a long time.\n> It is very nice that it gives us ticks (i.e. reproducible object\n> names) automatically and reduces the chance for new tests to forget\n> to do so, but I found it extremely annoying that it wants to always\n> add a light-weight tag to every commit it creates.\n> \n> I suspect that this design decision largely comes from the fact that\n> back then we did not exactly trust the syntax like \"master~3\" and\n> wanted to have a set of fixed points, but often these auto-generated\n> tags get in the way by either contaminating the refs namespaces (and\n> we are testing pull here, which worries me that test authors need to\n> be aware that these tags may be transferred to the destination) or\n> anchoring the objects in the object database (e.g. when writing\n> \"reset --hard branch branch~3 && prune\" tests, you need to remember\n> to kill these tags).\n> \n> So I dunno. I really wish test_commit didn't create tags and either\n> left the tagging to the calling script.\n\nYeah, light-weight tags with `test_commit` can be very annoying. IIRC the reason was not because we did not trust master~3, but that it is really unreadable. You have to be a Git nerd to find those tests intuitive that refer to master~3 which later is master~17 and then after a `git reset HEAD~5^2~3` is master~1 instead. The idea was that you could refer to the commits in question by their commit message.\n\nI guess that I should clean up this mess and remove the tagging facility. We could use the :/ syntax instead, for example.\n\nOr maybe just add a --tag flag to `test_commit` and use that in all cases where the tags were actually needed. Yeah, I think I like that option best.\n\nCiao,\nDscho\n"},{"id":"261552","messageId":"CAPc5daVze4+8aLGPpZgxDnvKSwvQiaR=kRdwfAHXFYA7HChmMg@mail.gmail.com","threadId":"39354","inReplyTo":"6b905c01c9b57abc05fb49117c28c10e@www.dscho.org","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-19T13:57:02Z","receivedAt":"2015-05-19T13:57:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Tue, May 19, 2015 at 6:35 AM, Johannes Schindelin\n wrote:\n>\n> On 2015-05-18 20:18, Junio C Hamano wrote:\n>>\n>> So I dunno. I really wish test_commit didn't create tags and either\n>> left the tagging to the calling script.\n>\n> Or maybe just add a --tag flag to `test_commit` and use that in all cases where the tags were actually needed. Yeah, I think I like that option best.\n\nThanks for inferring what I wanted to say but left unsaid due to my\nstupidity and\nlack of proofreading. I meant to follow \"either left to the caller\"\nwith \"or with an\noption\" ;-)\n"},{"id":"261597","messageId":"1432070690.14498.4.camel@kaarsemaker.net","threadId":"39354","inReplyTo":"CAPc5daVze4+8aLGPpZgxDnvKSwvQiaR=kRdwfAHXFYA7HChmMg@mail.gmail.com","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2015-05-19T21:24:50Z","receivedAt":"2015-05-19T21:24:50Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On di, 2015-05-19 at 06:57 -0700, Junio C Hamano wrote:\n> On Tue, May 19, 2015 at 6:35 AM, Johannes Schindelin\n> wrote:\n> >\n> > On 2015-05-18 20:18, Junio C Hamano wrote:\n> >>\n> >> So I dunno. I really wish test_commit didn't create tags and either\n> >> left the tagging to the calling script.\n> >\n> > Or maybe just add a --tag flag to `test_commit` and use that in all cases where the tags were actually needed. Yeah, I think I like that option best.\n> \n> Thanks for inferring what I wanted to say but left unsaid due to my\n> stupidity and\n> lack of proofreading. I meant to follow \"either left to the caller\"\n> with \"or with an\n> option\" ;-)\n\nI took a stab at this, adding a --tag option to test_commit and adding\nthe option to the test_commit calls that need it (or removing tests'\nreliance on these tags where appropriate, or removing tests' workarounds\nfor dealing with these tags when they don't want them), and the result\nis 59 files changed, 280 insertions(+), 281 deletions(-)\n\nA test run on master with GIT_TEST_LONG set causes 1138 calls to\ntest_commit on my system, of which 255 now use the --tag option\n(measured with a really crude hack that INCR's some keys in redis at\nappropriate points in test_commit).\n\nIs this interesting enough to turn into a proper patch series?\n-- \nDennis Kaarsemaker\nwww.kaarsemaker.net\n"},{"id":"261598","messageId":"CAGZ79kaw_6Cq5SaLvJHrXDm9bh3+ovAddv9CQJ78m65MYF=SqA@mail.gmail.com","threadId":"39354","inReplyTo":"1432070690.14498.4.camel@kaarsemaker.net","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-05-19T21:33:17Z","receivedAt":"2015-05-19T21:33:17Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, May 19, 2015 at 2:24 PM, Dennis Kaarsemaker\n wrote:\n> On di, 2015-05-19 at 06:57 -0700, Junio C Hamano wrote:\n>> On Tue, May 19, 2015 at 6:35 AM, Johannes Schindelin\n>> wrote:\n>> >\n>> > On 2015-05-18 20:18, Junio C Hamano wrote:\n>> >>\n>> >> So I dunno. I really wish test_commit didn't create tags and either\n>> >> left the tagging to the calling script.\n>> >\n>> > Or maybe just add a --tag flag to `test_commit` and use that in all cases where the tags were actually needed. Yeah, I think I like that option best.\n>>\n>> Thanks for inferring what I wanted to say but left unsaid due to my\n>> stupidity and\n>> lack of proofreading. I meant to follow \"either left to the caller\"\n>> with \"or with an\n>> option\" ;-)\n>\n> I took a stab at this, adding a --tag option to test_commit and adding\n> the option to the test_commit calls that need it (or removing tests'\n> reliance on these tags where appropriate, or removing tests' workarounds\n> for dealing with these tags when they don't want them), and the result\n> is 59 files changed, 280 insertions(+), 281 deletions(-)\n\nI guess most of the line changes are just adding the --tag?\nAnd I'd guess (281-280) that there is no huge code inside of test_commit\neither, so I'd assume it doesn't add clutter, but rather cleans up....\n\n>\n> A test run on master with GIT_TEST_LONG set causes 1138 calls to\n> test_commit on my system, of which 255 now use the --tag option\n> (measured with a really crude hack that INCR's some keys in redis at\n> appropriate points in test_commit).\n\n... 255 out of 1138 is awesome IMHO! Do you see an improvement in time as\nwell (as in \"time make test\" is X% faster overall) ?\n\n>\n> Is this interesting enough to turn into a proper patch series?\n\nI'd think this is worth making a real patch, definitely!\n\nThanks,\nStefan\n"},{"id":"261599","messageId":"xmqqfv6s6ygb.fsf@gitster.dls.corp.google.com","threadId":"39354","inReplyTo":"1432070690.14498.4.camel@kaarsemaker.net","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-19T21:37:24Z","receivedAt":"2015-05-19T21:37:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dennis Kaarsemaker writes:\n\n> I took a stab at this, adding a --tag option to test_commit and adding\n> the option to the test_commit calls that need it (or removing tests'\n> reliance on these tags where appropriate, or removing tests' workarounds\n> for dealing with these tags when they don't want them), and the result\n> is 59 files changed, 280 insertions(+), 281 deletions(-)\n>\n> A test run on master with GIT_TEST_LONG set causes 1138 calls to\n> test_commit on my system, of which 255 now use the --tag option\n> (measured with a really crude hack that INCR's some keys in redis at\n> appropriate points in test_commit).\n>\n> Is this interesting enough to turn into a proper patch series?\n\nWow.\n\nA proper patch series would probably be\n\n [1/N] Teach \"test_commit --tag\" and replace existing \"test_commit\"\n with \"test_commit --tag\"\n\n [2-N/N] For all the test scripts, analyse and judge if they are\n better off with the auto-generated tags (i.e. no change wrt\n the result of 1/N) or tags that are created by the script\n at strategic places only as needed, and convert those that\n are better read without \"test_commit --tag\".\n\n[1/N] would be mechanical and easy, but justifying the change in the\nremainder would be a lot of work and reviewing would be, too, and\nwould require a good taste.\n\nPerhaps if we see two sample patches to see how it looks like, would\nthat help us decide?\n\nThat is, the mechanical [1/N] and [2/N] for one of the test script\nthat can do without --tag, and a sample \"do not apply\" patch to show\n\"if we change 'test_commit --tag' to 'test_commit', the script t1234\nneeds this many manual tagging by the caller, and it is not worth\ndoing\"? I dunno.\n\nThanks.\n"},{"id":"261601","messageId":"1432071832.14498.6.camel@kaarsemaker.net","threadId":"39354","inReplyTo":"CAGZ79kaw_6Cq5SaLvJHrXDm9bh3+ovAddv9CQJ78m65MYF=SqA@mail.gmail.com","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2015-05-19T21:43:52Z","receivedAt":"2015-05-19T21:43:52Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On di, 2015-05-19 at 14:33 -0700, Stefan Beller wrote:\n> On Tue, May 19, 2015 at 2:24 PM, Dennis Kaarsemaker\n> wrote:\n> > On di, 2015-05-19 at 06:57 -0700, Junio C Hamano wrote:\n> >> On Tue, May 19, 2015 at 6:35 AM, Johannes Schindelin\n> >> wrote:\n> >> >\n> >> > On 2015-05-18 20:18, Junio C Hamano wrote:\n> >> >>\n> >> >> So I dunno. I really wish test_commit didn't create tags and either\n> >> >> left the tagging to the calling script.\n> >> >\n> >> > Or maybe just add a --tag flag to `test_commit` and use that in all cases where the tags were actually needed. Yeah, I think I like that option best.\n> >>\n> >> Thanks for inferring what I wanted to say but left unsaid due to my\n> >> stupidity and\n> >> lack of proofreading. I meant to follow \"either left to the caller\"\n> >> with \"or with an\n> >> option\" ;-)\n> >\n> > I took a stab at this, adding a --tag option to test_commit and adding\n> > the option to the test_commit calls that need it (or removing tests'\n> > reliance on these tags where appropriate, or removing tests' workarounds\n> > for dealing with these tags when they don't want them), and the result\n> > is 59 files changed, 280 insertions(+), 281 deletions(-)\n> \n> I guess most of the line changes are just adding the --tag?\n> And I'd guess (281-280) that there is no huge code inside of test_commit\n> either, so I'd assume it doesn't add clutter, but rather cleans up....\n\nFull stat:\n\n t/t0100-previous.sh | 6 +++---\n t/t1011-read-tree-sparse-checkout.sh | 2 +-\n t/t1403-show-ref.sh | 6 +++---\n t/t1430-bad-ref-name.sh | 2 +-\n t/t1450-fsck.sh | 4 ++--\n t/t2018-checkout-branch.sh | 4 ++--\n t/t2020-checkout-detach.sh | 8 ++++----\n t/t2030-unresolve-info.sh | 10 +++++-----\n t/t3200-branch.sh | 2 +-\n t/t3211-peel-ref.sh | 2 +-\n t/t3307-notes-man.sh | 4 ++--\n t/t3308-notes-merge.sh | 10 +++++-----\n t/t3309-notes-merge-auto-resolve.sh | 30\n+++++++++++++++---------------\n t/t3310-notes-merge-manual-resolve.sh | 10 +++++-----\n t/t3311-notes-merge-fanout.sh | 2 +-\n t/t3400-rebase.sh | 6 +++---\n t/t3404-rebase-interactive.sh | 22 +++++++++++-----------\n t/t3406-rebase-message.sh | 6 +++---\n t/t3410-rebase-preserve-dropped-merges.sh | 14 +++++++-------\n t/t3411-rebase-preserve-around-merges.sh | 14 +++++++-------\n t/t3414-rebase-preserve-onto.sh | 14 +++++++-------\n t/t3416-rebase-onto-threedots.sh | 12 ++++++------\n t/t3418-rebase-continue.sh | 4 ++--\n t/t3421-rebase-topology-linear.sh | 22 +++++++++++-----------\n t/t3425-rebase-topology-merges.sh | 20 ++++++++++----------\n t/t3507-cherry-pick-conflict.sh | 8 ++++----\n t/t3508-cherry-pick-many-commits.sh | 6 +++---\n t/t3510-cherry-pick-sequence.sh | 16 ++++++++--------\n t/t3511-cherry-pick-x.sh | 2 +-\n t/t4131-apply-fake-ancestor.sh | 8 ++++----\n t/t4152-am-subjects.sh | 2 +-\n t/t4202-log.sh | 14 +++++++-------\n t/t4207-log-decoration-colors.sh | 8 +++-----\n t/t4300-merge-tree.sh | 62 +++++++++++++++++++++++++++++---------------------------------\n t/t5407-post-rewrite-hook.sh | 12 ++++++------\n t/t5509-fetch-push-namespaces.sh | 4 ++--\n t/t5510-fetch.sh | 4 ++--\n t/t5514-fetch-multiple.sh | 2 +-\n t/t5571-pre-push-hook.sh | 2 +-\n t/t5704-bundle.sh | 5 +----\n t/t6009-rev-list-parent.sh | 18 +++++++++---------\n t/t6010-merge-base.sh | 24 ++++++++++++------------\n t/t6016-rev-list-graph-simplify-history.sh | 22 +++++++++++-----------\n t/t6019-rev-list-ancestry-path.sh | 18 +++++++++---------\n t/t6101-rev-parse-parents.sh | 8 ++++----\n t/t6111-rev-list-treesame.sh | 2 +-\n t/t6300-for-each-ref.sh | 1 -\n t/t7003-filter-branch.sh | 16 ++++++++--------\n t/t7007-show.sh | 8 ++++----\n t/t7012-skip-worktree-writing.sh | 2 +-\n t/t7060-wtstatus.sh | 6 +++---\n t/t7111-reset-table.sh | 6 +++---\n t/t7407-submodule-foreach.sh | 2 +-\n t/t7512-status-help.sh | 6 +++---\n t/t7606-merge-custom.sh | 8 ++++----\n t/t7607-merge-overwrite.sh | 6 +++---\n t/t7608-merge-messages.sh | 4 ++--\n t/t9502-gitweb-standalone-parse-output.sh | 2 +-\n t/test-lib-functions.sh | 9 ++++++++-\n 59 files changed, 278 insertions(+), 281 deletions(-)\n\n> > A test run on master with GIT_TEST_LONG set causes 1138 calls to\n> > test_commit on my system, of which 255 now use the --tag option\n> > (measured with a really crude hack that INCR's some keys in redis at\n> > appropriate points in test_commit).\n> \n> ... 255 out of 1138 is awesome IMHO! Do you see an improvement in time as\n> well (as in \"time make test\" is X% faster overall) ?\n\nI have not measured, but will.\n\n> > Is this interesting enough to turn into a proper patch series?\n> \n> I'd think this is worth making a real patch, definitely!\n\nOK.\n-- \nDennis Kaarsemaker\nwww.kaarsemaker.net\n"},{"id":"261606","messageId":"1432072167.14498.12.camel@kaarsemaker.net","threadId":"39354","inReplyTo":"xmqqfv6s6ygb.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2015-05-19T21:49:27Z","receivedAt":"2015-05-19T21:49:27Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On di, 2015-05-19 at 14:37 -0700, Junio C Hamano wrote:\n> Dennis Kaarsemaker writes:\n> \n> > I took a stab at this, adding a --tag option to test_commit and adding\n> > the option to the test_commit calls that need it (or removing tests'\n> > reliance on these tags where appropriate, or removing tests' workarounds\n> > for dealing with these tags when they don't want them), and the result\n> > is 59 files changed, 280 insertions(+), 281 deletions(-)\n> >\n> > A test run on master with GIT_TEST_LONG set causes 1138 calls to\n> > test_commit on my system, of which 255 now use the --tag option\n> > (measured with a really crude hack that INCR's some keys in redis at\n> > appropriate points in test_commit).\n> >\n> > Is this interesting enough to turn into a proper patch series?\n> \n> Wow.\n> \n> A proper patch series would probably be\n> \n> [1/N] Teach \"test_commit --tag\" and replace existing \"test_commit\"\n> with \"test_commit --tag\"\n> \n> [2-N/N] For all the test scripts, analyse and judge if they are\n> better off with the auto-generated tags (i.e. no change wrt\n> the result of 1/N) or tags that are created by the script\n> at strategic places only as needed, and convert those that\n> are better read without \"test_commit --tag\".\n> \n> [1/N] would be mechanical and easy, but justifying the change in the\n> remainder would be a lot of work and reviewing would be, too, and\n> would require a good taste.\n\nI've actually done it differently while implementing:\n\n1) Make test_commit recognize --tags and stop creating tags unless\n specified\n2) while ! prove --state=save,failed {\n Find and fix tests that now need --tags\n }\n\nFor the actual patch series I'll add -p the changes slightly\ndifferently:\n\n1/N: Make test_commit recognize a --tags parameter but not change\nbehaviour.\n2/N - N-1/N: Add --tags where necesary (or other fixes as appropriate)\nN/N: Only write tags when --tags is passed to test_commit.\n\nThat way 'make test' will pass at every step.\n\n> Perhaps if we see two sample patches to see how it looks like, would\n> that help us decide?\n> \n> That is, the mechanical [1/N] and [2/N] for one of the test script\n> that can do without --tag, and a sample \"do not apply\" patch to show\n> \"if we change 'test_commit --tag' to 'test_commit', the script t1234\n> needs this many manual tagging by the caller, and it is not worth\n> doing\"? I dunno.\n\nI'll just send an entire patch series. It's not that much more work.\n\n-- \nDennis Kaarsemaker\nwww.kaarsemaker.net\n"},{"id":"261610","messageId":"xmqqmw10p6ad.fsf@gitster.dls.corp.google.com","threadId":"39354","inReplyTo":"1432072167.14498.12.camel@kaarsemaker.net","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-19T22:10:50Z","receivedAt":"2015-05-19T22:10:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dennis Kaarsemaker writes:\n\n> I've actually done it differently while implementing:\n>\n> 1) Make test_commit recognize --tags and stop creating tags unless\n> specified\n> 2) while ! prove --state=save,failed {\n> Find and fix tests that now need --tags\n> }\n\nThat was what I feared. The result of that process is the hardest\nto reason about and review.\n\n> For the actual patch series I'll add -p the changes slightly\n> differently:\n>\n> 1/N: Make test_commit recognize a --tags parameter but not change\n> behaviour.\n\nMake test will pass after this one.\n\n> 2/N - N-1/N: Add --tags where necesary (or other fixes as appropriate)\n> N/N: Only write tags when --tags is passed to test_commit.\n\nAnd untouched ones will continue passing.\n\nOnly when you remove --tags from ones that need (i.e. the caller\nneeds to create the necessary tags), we will see a patch and unless\nthere is mistake in the conversion, the result should pass.\n"},{"id":"261622","messageId":"1432074626.14498.16.camel@kaarsemaker.net","threadId":"39354","inReplyTo":"xmqqmw10p6ad.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2015-05-19T22:30:26Z","receivedAt":"2015-05-19T22:30:26Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On di, 2015-05-19 at 15:10 -0700, Junio C Hamano wrote:\n> Dennis Kaarsemaker writes:\n> \n> > I've actually done it differently while implementing:\n> >\n> > 1) Make test_commit recognize --tags and stop creating tags unless\n> > specified\n> > 2) while ! prove --state=save,failed {\n> > Find and fix tests that now need --tags\n> > }\n> \n> That was what I feared. The result of that process is the hardest\n> to reason about and review.\n> \n> > For the actual patch series I'll add -p the changes slightly\n> > differently:\n> >\n> > 1/N: Make test_commit recognize a --tags parameter but not change\n> > behaviour.\n> \n> Make test will pass after this one.\n> \n> > 2/N - N-1/N: Add --tags where necesary (or other fixes as appropriate)\n> > N/N: Only write tags when --tags is passed to test_commit.\n> \n> And untouched ones will continue passing.\n>\n> Only when you remove --tags from ones that need (i.e. the caller\n> needs to create the necessary tags), we will see a patch and unless\n> there is mistake in the conversion, the result should pass.\n\nI'm not quite sure I understand what you're trying to say. You seem to\nbe worried that there will be silent successes for tests that should\nfail after N/N if I take the proposed approach. I have no idea how that\ncould happen though.\n\n-- \nDennis Kaarsemaker\nwww.kaarsemaker.net\n"},{"id":"261625","messageId":"CAPc5daUF_yotXVf_=CFz_q1GBXVdq8pJxeuNuv=BAC54TJVT9w@mail.gmail.com","threadId":"39354","inReplyTo":"1432074626.14498.16.camel@kaarsemaker.net","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-19T23:14:03Z","receivedAt":"2015-05-19T23:14:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Tue, May 19, 2015 at 3:30 PM, Dennis Kaarsemaker\n wrote:\n> On di, 2015-05-19 at 15:10 -0700, Junio C Hamano wrote:\n>> Dennis Kaarsemaker writes:\n>>\n>> > I've actually done it differently while implementing:\n>> >\n>> > 1) Make test_commit recognize --tags and stop creating tags unless\n>> > specified\n>> > 2) while ! prove --state=save,failed {\n>> > Find and fix tests that now need --tags\n>> > }\n>>\n>> That was what I feared. The result of that process is the hardest\n>> to reason about and review.\n>>\n>\n> I'm not quite sure I understand what you're trying to say. You seem to\n> be worried that there will be silent successes for tests that should\n> fail after N/N if I take the proposed approach. I have no idea how that\n> could happen though.\n\nYou are placing too much faith in the tests. We would want to see a\nconversion that\nis reasonable and indeed reasoned by eyeballing the change for correctness.\n\nBesides, there are different build options and test configurations. Even if you\nrun with EXPENSIVE defined, the only thing we can say without validating the\nchanges is the test happened to have passed. There is no assurance that the\nresult of your change will catch the same breakage, if inttroduced later to the\ncode (not tests), as the current unmodified tests will. Two mistakes may happen\nto hide the breakage in tests. Taking to the extreme, if you replace\nall the test\nbodies in test_expect_success with \"true\", the result of such a\nrewrite will still\npass all the existting tests, and the only way you can say \"That's absurd\" is\nbecause you compare what the existing tests do and a single \"true\" would do\nand realize that they do vastly different things. Your \"while ! prove\n...\" method\ndoes not give me much more faith than such a \"rewrite everything to true without\nlooking\" method.\n"},{"id":"261628","messageId":"xmqqegmcour4.fsf@gitster.dls.corp.google.com","threadId":"39354","inReplyTo":"CAPc5daUF_yotXVf_=CFz_q1GBXVdq8pJxeuNuv=BAC54TJVT9w@mail.gmail.com","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-20T02:19:59Z","receivedAt":"2015-05-20T02:19:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> On Tue, May 19, 2015 at 3:30 PM, Dennis Kaarsemaker\n> wrote:\n>> On di, 2015-05-19 at 15:10 -0700, Junio C Hamano wrote:\n>>> Dennis Kaarsemaker writes:\n>>>\n>>> > I've actually done it differently while implementing:\n>>> >\n>>> > 1) Make test_commit recognize --tags and stop creating tags unless\n>>> > specified\n>>> > 2) while ! prove --state=save,failed {\n>>> > Find and fix tests that now need --tags\n>>> > }\n>>>\n>>> That was what I feared. The result of that process is the hardest\n>>> to reason about and review.\n>>\n>> I'm not quite sure I understand what you're trying to say. You seem to\n>> be worried that there will be silent successes for tests that should\n>> fail after N/N if I take the proposed approach. I have no idea how that\n>> could happen though.\n>\n> You are placing too much faith in the tests. We would want to see\n> a conversion that is reasonable and indeed reasoned by eyeballing\n> the change for correctness.\n\nTo put it in another way, imagine this scenario.\n\nSix months ago, you found a bug in \"git frotz\" command. The bug\ntriggered only when a commit had an extra tag pointing at it. You\nfixed the bug, and in order to protect your fix from getting broken\nby other people, added a new test that would have failed with the\nbug present, perhaps like this:\n\n test_expect_success 'commit with an extra tag' '\n\ttest_commit sample &&\n git frotz HEAD\n '\n\nFast forward to today. You changed \"test_commit\" and it no longer\ncreates a tag by default with your step 1.\n\nYou run this test again.\n\nIt will pass.\n\nWhy? \"git frotz\" command used to break when you had tag, but did\nnot break if you didn't have the tag even without your bugfix six\nmonths ago.\n\nNext week, somebody will accidentally break the bugfix you did six\nmonths ago. \"git frotz\" again does not work correctly when the\ncommit is pointed at by a tag. But with the test will not catch\nthis breakage.\n\nThe problem was that your step 1 broke this test, and your step 2\nof running the test did not catch the fact that you broke it.\n\nHopefully now you have some idea how your approach is problematic.\n"},{"id":"261632","messageId":"xmqqzj4zomu6.fsf@gitster.dls.corp.google.com","threadId":"39354","inReplyTo":"xmqqfv6s6ygb.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-20T05:10:57Z","receivedAt":"2015-05-20T05:10:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> A proper patch series would probably be\n>\n> [1/N] Teach \"test_commit --tag\" and replace existing \"test_commit\"\n> with \"test_commit --tag\"\n>\n> [2-N/N] For all the test scripts, analyse and judge if they are\n> better off with the auto-generated tags (i.e. no change wrt\n> the result of 1/N) or tags that are created by the script\n> at strategic places only as needed, and convert those that\n> are better read without \"test_commit --tag\".\n>\n> [1/N] would be mechanical and easy, but justifying the change in the\n> remainder would be a lot of work and reviewing would be, too, and\n> would require a good taste.\n\nActually even the above is not sufficient, as we have to worry about\ntopics in-flight (aka \"the rest of the world is not standing still\nwaiting for you\"). It must go more like this:\n\n [1/N] Introduce a new helper (let's call it \"make_commit\") that\n by default does not do too much but optionally can do as much\n as the \"test_commit\" does. Reimplement \"test_commit\" in\n terms of \"make_commit\"\n\n [2-N/N] Exaine existing uses of \"test_commit\" and reduce what they\n do by rewriting them with \"make_commit\" as needed.\n\nSo I'd say 1/N may probably be something worth doing, but I am not\nconvinced that the rest is a good use of reviewer's time.\n\nSpeaking of \"does not do too much\", I think the behaviour of\n\"test_commit\" that always adds new a file outlived its usefulness.\n\nWhen a test is primarily interested in the topology of the resulting\nhistory (most \"log\" tests are), we can use \"commit --allow-empty\"\nthese days (I think \"test_commit\" predates it and adding a file may\nhave been a handy way to ensure that the new commit's tree is\ndifferent from parent's). And when a test is primarily about how\nbranches with different contents in various paths interact with each\nother (e.g. checking out, merging), it does not make a very useful\ntest vector to work on if you can only build your tests with a set\nof branches whose differences can only be creation and deletion but\nnever in-place modification of paths.\n\nAmong various potential uses of the hypothetical \"test_commit\"\nreplacement that I would imagine, what is common is only that it\ndoes the \"tick\" thing and then it creates a commit, and nothing\nelse. Everything else is optional and needs customization. But\nthen there is not much left, i.e.\n\n make_commit () {\n\ttest_tick && git commit --allow-empty \"$@\"\n }\n\nwhich may be useful to avoid forgetting the tick thing, but does not\nhelp much else.\n\nEven if that were found to be useful, this discussion is only for\nstep 1/N; let's refrain from rewriting everything for the sake of\nrewriting. I do not think it would be a good use of developer time\nor reviewer time.\n"},{"id":"261643","messageId":"1432109506.18433.0.camel@kaarsemaker.net","threadId":"39354","inReplyTo":"xmqqegmcour4.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2015-05-20T08:11:46Z","receivedAt":"2015-05-20T08:11:46Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On di, 2015-05-19 at 19:19 -0700, Junio C Hamano wrote:\n> Hopefully now you have some idea how your approach is problematic.\n\nYes, thanks for the thorough explanation!\n\nTwo more bad sideeffects, so two more reasons not to take this approach:\n\n- test_must_fail tests might now fail for the wrong reason, undetectedly\n (\"ref does not exist\" instead of \"can't handle ref\")\n- same for TODO tests\n\n-- \nDennis Kaarsemaker\nwww.kaarsemaker.net\n"},{"id":"261644","messageId":"1432109616.18433.2.camel@kaarsemaker.net","threadId":"39354","inReplyTo":"xmqqzj4zomu6.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] pull: handle --log=","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2015-05-20T08:13:36Z","receivedAt":"2015-05-20T08:13:36Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On di, 2015-05-19 at 22:10 -0700, Junio C Hamano wrote:\n> Even if that were found to be useful, this discussion is only for\n> step 1/N; let's refrain from rewriting everything for the sake of\n> rewriting. I do not think it would be a good use of developer time\n> or reviewer time.\n\nAgreed, and in combination with the very marginal (if any) time saving,\nI'll just drop the whole thing.\n\n-- \nDennis Kaarsemaker\nwww.kaarsemaker.net\n"},{"id":"261772","messageId":"20150521103617.GA6159@yoshi.chippynet.com","threadId":"39354","inReplyTo":"5661061272076a1883cfde1087be4a42@www.dscho.org","subject":"[PATCH v3] pull: handle --log=","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-21T10:36:17Z","receivedAt":"2015-05-21T10:36:17Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Mon, May 18, 2015 at 10:53 PM, Johannes Schindelin wrote:\n> On 2015-05-18 15:39, Paul Tan wrote:\n>> diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh\n>> index 8cccecc..eebb8c9 100755\n>> --- a/t/t5524-pull-msg.sh\n>> +++ b/t/t5524-pull-msg.sh\n>> @@ -17,6 +17,9 @@ test_expect_success setup '\n>> git commit -m \"add bfile\"\n>> ) &&\n>> test_tick && test_tick &&\n>> + echo \"second\" >afile &&\n>> + git add afile &&\n>> + git commit -m \"second commit\" &&\n>> echo \"original $dollar\" >afile &&\n>> git add afile &&\n>\n> I completely forgot to send a mail last time where I wanted to mention the test_commit function. I *think* that above invocation is equivalent to\n>\n> test_commit \"second commit\" afile second\n>\n> with the additional benefit that `test_tick` is called automatically. Maybe you want to use that more concise function call?\n\nHmm, it says that 'second commit' is not a valid tag name ;-) Okay, I can work around it by making the commit message \"second\".\n\nOn another note though, I think I need to add a \"test_tick\" for the commit just below as well:\n\n echo \"original $dollar\" >afile &&\n git add afile &&c\n git commit -m \"do not clobber $dollar signs\"\n\nSo, here's the re-rolled patch.\n\n--- >8 ---\nSince efb779f (merge, pull: add '--(no-)log' command line option,\n2008-04-06) git-pull supported the (--no-)log switch and would pass it\nto git-merge.\n\n96e9420 (merge: Make '--log' an integer option for number of shortlog\nentries, 2010-09-08) implemented support for the --log= switch, which\nwould explicitly set the number of shortlog entries. However, git-pull\ndoes not recognize this option, and will instead pass it to git-fetch,\nleading to \"unknown option\" errors.\n\nFix this by matching --log=* in addition to --log and --no-log.\n\nImplement a test for this use case.\n\nSigned-off-by: Paul Tan \n---\n git-pull.sh | 4 ++--\n t/t5524-pull-msg.sh | 18 +++++++++++++++++-\n 2 files changed, 19 insertions(+), 3 deletions(-)\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 9ed01fd..5ff4545 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -81,8 +81,8 @@ do\n \t\tdiffstat=--no-stat ;;\n \t--stat|--summary)\n \t\tdiffstat=--stat ;;\n-\t--log|--no-log)\n-\t\tlog_arg=$1 ;;\n+\t--log|--log=*|--no-log)\n+\t\tlog_arg=\"$1\" ;;\n \t--no-c|--no-co|--no-com|--no-comm|--no-commi|--no-commit)\n \t\tno_commit=--no-commit ;;\n \t--c|--co|--com|--comm|--commi|--commit)\ndiff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh\nindex 8cccecc..9bc7821 100755\n--- a/t/t5524-pull-msg.sh\n+++ b/t/t5524-pull-msg.sh\n@@ -16,7 +16,9 @@ test_expect_success setup '\n \t\ttest_tick &&\n \t\tgit commit -m \"add bfile\"\n \t) &&\n-\ttest_tick && test_tick &&\n+\ttest_tick &&\n+\ttest_commit second afile &&\n+\ttest_tick &&\n \techo \"original $dollar\" >afile &&\n \tgit add afile &&\n \tgit commit -m \"do not clobber $dollar signs\"\n@@ -32,4 +34,18 @@ test_expect_success pull '\n )\n '\n \n+test_expect_success '--log=1 limits shortlog length' '\n+(\n+\tcd cloned &&\n+\tgit reset --hard HEAD^ &&\n+\ttest \"$(cat afile)\" = original &&\n+\ttest \"$(cat bfile)\" = added &&\n+\tgit pull --log=1 &&\n+\tgit log -3 &&\n+\tgit cat-file commit HEAD >result &&\n+\tgrep Dollar result &&\n+\t! grep second result\n+)\n+'\n+\n test_done\n-- \n2.1.4\n"},{"id":"261855","messageId":"xmqq1ti9ipy9.fsf@gitster.dls.corp.google.com","threadId":"39354","inReplyTo":"20150521103617.GA6159@yoshi.chippynet.com","subject":"Re: [PATCH v3] pull: handle --log=","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-21T21:24:46Z","receivedAt":"2015-05-21T21:24:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan writes:\n\n> So, here's the re-rolled patch.\n\nSigh, too late.\n\nI thought the previous round was good enough and the patch is\nalready on 'next'.\n\nIf the incremental change is still worth doing on top, please do so.\n\nThanks.\n"},{"id":"261902","messageId":"CACRoPnSbAA=6xH+4W8KGPRWe8Pi304zz70Kz8TgY92qKtDYQ8A@mail.gmail.com","threadId":"39354","inReplyTo":"xmqq1ti9ipy9.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3] pull: handle --log=","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-05-22T13:29:55Z","receivedAt":"2015-05-22T13:29:55Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi Junio,\n\nOn Fri, May 22, 2015 at 5:24 AM, Junio C Hamano wrote:\n> Paul Tan writes:\n>\n>> So, here's the re-rolled patch.\n>\n> Sigh, too late.\n>\n> I thought the previous round was good enough and the patch is\n> already on 'next'.\n>\n> If the incremental change is still worth doing on top, please do so.\n>\n> Thanks.\n\nMy bad, I should have checked 'next'.\n\nBut it's okay, I re-rolled the patch because it seemed that people\npreferred \"test_commit\". I personally don't have a strong opinion on\nthis, so if you're fine with it then I'm okay as well.\n\nThanks,\nPaul\n"}]}