{"thread":{"id":"39185","subject":"Diffing submodule does not yield complete logs for merge commits","startedAt":"2015-04-29T20:53:11Z","lastAt":"2015-06-02T12:02:15Z","messageCount":24,"participants":["Robert Dailey","Heiko Voigt","Jens Lehmann","Junio C Hamano","Johannes Schindelin","Stefan Beller","Roberto Tyley"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"260201","messageId":"CAHd499BqB_ZFKMNxSVCDTFx2Ge=TfCE6gexFn+rfRbS+ybLybA@mail.gmail.com","threadId":"39185","inReplyTo":null,"subject":"Diffing submodule does not yield complete logs for merge commits","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2015-04-29T20:53:11Z","receivedAt":"2015-04-29T20:53:11Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"I am attempting to diff a submodule modified in my working copy and\nthe only difference is a merge commit. However, I do not get the\n\"full\" range of commits introduced by the merge commit when I diff it:\n\n$ git diff --submodule=log Core\nSubmodule Core 8b4ec60..def2f3b:\n  > Merge remote-tracking branch 'origin/master-ah3k'\n\nHowever if I go inside my submodule and run `git log` by hand, I get\nmore information about the TRUE commits introduced:\n\n$ git log --oneline 8b4ec60..def2f3b\ndef2f3b Merge remote-tracking branch 'origin/master-ah3k'\n015c961 Remove log spam in FontManager\n7713ba1 Update third party submodule to latest\n10aac78 Merge pull request #9 in FE/core from\nfeature/FE-1348-selecting-continue-on-zero-balance to master-ah3k\n287882f FE-1376 Nedd to remain in check detail screen when selecting\ndonation after SBI\na5a6bed Do not overwrite the current check# within loop\ndfb8547 Adding list of checks to CRspChecks before saving\n1be280a FE-1354: Guest logged out in specific multiple check scenario\nde06d5a [FE-1348] Fix PATT exit while checks still open\n\nIt's almost as if the `git diff --submodule=log` approach is passing\nin --first-parent to git log, which would exclude commits in the range\nthat I'm seeing when I run git log manually.\n\nIs this by design? Is there a way to enable the full log history with\n`git diff` on a submodule?\n"},{"id":"260365","messageId":"20150501175757.GA10569@book.hvoigt.net","threadId":"39185","inReplyTo":"CAHd499BqB_ZFKMNxSVCDTFx2Ge=TfCE6gexFn+rfRbS+ybLybA@mail.gmail.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2015-05-01T17:57:57Z","receivedAt":"2015-05-01T17:57:57Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Wed, Apr 29, 2015 at 03:53:11PM -0500, Robert Dailey wrote:\n> I am attempting to diff a submodule modified in my working copy and\n> the only difference is a merge commit. However, I do not get the\n> \"full\" range of commits introduced by the merge commit when I diff it:\n> \n> $ git diff --submodule=log Core\n> Submodule Core 8b4ec60..def2f3b:\n>   > Merge remote-tracking branch 'origin/master-ah3k'\n> \n> However if I go inside my submodule and run `git log` by hand, I get\n> more information about the TRUE commits introduced:\n> \n> $ git log --oneline 8b4ec60..def2f3b\n> def2f3b Merge remote-tracking branch 'origin/master-ah3k'\n> 015c961 Remove log spam in FontManager\n> 7713ba1 Update third party submodule to latest\n> 10aac78 Merge pull request #9 in FE/core from\n> feature/FE-1348-selecting-continue-on-zero-balance to master-ah3k\n> 287882f FE-1376 Nedd to remain in check detail screen when selecting\n> donation after SBI\n> a5a6bed Do not overwrite the current check# within loop\n> dfb8547 Adding list of checks to CRspChecks before saving\n> 1be280a FE-1354: Guest logged out in specific multiple check scenario\n> de06d5a [FE-1348] Fix PATT exit while checks still open\n> \n> It's almost as if the `git diff --submodule=log` approach is passing\n> in --first-parent to git log, which would exclude commits in the range\n> that I'm seeing when I run git log manually.\n\nThat is exactly the case. In prepare_submodule_summary() that option is\nset before doing the revision walk.\n\n> Is this by design? Is there a way to enable the full log history with\n> `git diff` on a submodule?\n\nThis stems from the first implementation for showing submodule diffs in\ncommit 752c0c24. I guess this was done deliberately to limit the amount\nof output you get for a submodule. At the moment this is hardcoded but I\nthink there is nothing wrong with adding another option to include the\nfull log.\n\nCheers Heiko\n"},{"id":"260527","messageId":"CAHd499B=EcgYiTMFt9VYhj45bRkP8h9TBk1B0cr8fYFuXNe_mQ@mail.gmail.com","threadId":"39185","inReplyTo":"20150501175757.GA10569@book.hvoigt.net","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2015-05-04T15:05:22Z","receivedAt":"2015-05-04T15:05:22Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Fri, May 1, 2015 at 12:57 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> Hi,\n>\n> On Wed, Apr 29, 2015 at 03:53:11PM -0500, Robert Dailey wrote:\n>> I am attempting to diff a submodule modified in my working copy and\n>> the only difference is a merge commit. However, I do not get the\n>> \"full\" range of commits introduced by the merge commit when I diff it:\n>>\n>> $ git diff --submodule=log Core\n>> Submodule Core 8b4ec60..def2f3b:\n>>   > Merge remote-tracking branch 'origin/master-ah3k'\n>>\n>> However if I go inside my submodule and run `git log` by hand, I get\n>> more information about the TRUE commits introduced:\n>>\n>> $ git log --oneline 8b4ec60..def2f3b\n>> def2f3b Merge remote-tracking branch 'origin/master-ah3k'\n>> 015c961 Remove log spam in FontManager\n>> 7713ba1 Update third party submodule to latest\n>> 10aac78 Merge pull request #9 in FE/core from\n>> feature/FE-1348-selecting-continue-on-zero-balance to master-ah3k\n>> 287882f FE-1376 Nedd to remain in check detail screen when selecting\n>> donation after SBI\n>> a5a6bed Do not overwrite the current check# within loop\n>> dfb8547 Adding list of checks to CRspChecks before saving\n>> 1be280a FE-1354: Guest logged out in specific multiple check scenario\n>> de06d5a [FE-1348] Fix PATT exit while checks still open\n>>\n>> It's almost as if the `git diff --submodule=log` approach is passing\n>> in --first-parent to git log, which would exclude commits in the range\n>> that I'm seeing when I run git log manually.\n>\n> That is exactly the case. In prepare_submodule_summary() that option is\n> set before doing the revision walk.\n>\n>> Is this by design? Is there a way to enable the full log history with\n>> `git diff` on a submodule?\n>\n> This stems from the first implementation for showing submodule diffs in\n> commit 752c0c24. I guess this was done deliberately to limit the amount\n> of output you get for a submodule. At the moment this is hardcoded but I\n> think there is nothing wrong with adding another option to include the\n> full log.\n>\n> Cheers Heiko\n\nI will go ahead and work on this feature. Here is what I'd like to see:\n\n1. `git diff --submodule` should have the ability to display full logs\nvs current logs (i.e. without --first-parent)\n2. `git submodule summary` should have an option to display full logs\nor \"first-parent\" logs.\n\nFor #1, do you recommend adding a 3rd setting for `diff.submodule`\nconfig? Something like \"full-log\" or something? Or an entirely new\nconfig? I noticed that in diff.h, the DIFF_OPT flags already consume\n31 bits. If this is a 32-bit flag, there is only 1 bit left. If we go\nwith a 3rd setting for `diff.submodule` I think this might consume the\nlast bit.\n\nWe could also make `git diff --submodule` default to the \"full log\"\ntype, and if users want only first parent logs in submodule summary,\nthey'd have to execute `git submodule summary` instead.\n\nThere are a few options. What do you recommend? Thanks.\n"},{"id":"260541","messageId":"5547C961.7070909@web.de","threadId":"39185","inReplyTo":"CAHd499B=EcgYiTMFt9VYhj45bRkP8h9TBk1B0cr8fYFuXNe_mQ@mail.gmail.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2015-05-04T19:32:49Z","receivedAt":"2015-05-04T19:32:49Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 04.05.2015 um 17:05 schrieb Robert Dailey:\n> On Fri, May 1, 2015 at 12:57 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n>> Hi,\n>>\n>> On Wed, Apr 29, 2015 at 03:53:11PM -0500, Robert Dailey wrote:\n>>> I am attempting to diff a submodule modified in my working copy and\n>>> the only difference is a merge commit. However, I do not get the\n>>> \"full\" range of commits introduced by the merge commit when I diff it:\n>>>\n>>> $ git diff --submodule=log Core\n>>> Submodule Core 8b4ec60..def2f3b:\n>>>    > Merge remote-tracking branch 'origin/master-ah3k'\n>>>\n>>> However if I go inside my submodule and run `git log` by hand, I get\n>>> more information about the TRUE commits introduced:\n>>>\n>>> $ git log --oneline 8b4ec60..def2f3b\n>>> def2f3b Merge remote-tracking branch 'origin/master-ah3k'\n>>> 015c961 Remove log spam in FontManager\n>>> 7713ba1 Update third party submodule to latest\n>>> 10aac78 Merge pull request #9 in FE/core from\n>>> feature/FE-1348-selecting-continue-on-zero-balance to master-ah3k\n>>> 287882f FE-1376 Nedd to remain in check detail screen when selecting\n>>> donation after SBI\n>>> a5a6bed Do not overwrite the current check# within loop\n>>> dfb8547 Adding list of checks to CRspChecks before saving\n>>> 1be280a FE-1354: Guest logged out in specific multiple check scenario\n>>> de06d5a [FE-1348] Fix PATT exit while checks still open\n>>>\n>>> It's almost as if the `git diff --submodule=log` approach is passing\n>>> in --first-parent to git log, which would exclude commits in the range\n>>> that I'm seeing when I run git log manually.\n>>\n>> That is exactly the case. In prepare_submodule_summary() that option is\n>> set before doing the revision walk.\n>>\n>>> Is this by design? Is there a way to enable the full log history with\n>>> `git diff` on a submodule?\n>>\n>> This stems from the first implementation for showing submodule diffs in\n>> commit 752c0c24. I guess this was done deliberately to limit the amount\n>> of output you get for a submodule. At the moment this is hardcoded but I\n>> think there is nothing wrong with adding another option to include the\n>> full log.\n>>\n>> Cheers Heiko\n>\n> I will go ahead and work on this feature. Here is what I'd like to see:\n>\n> 1. `git diff --submodule` should have the ability to display full logs\n> vs current logs (i.e. without --first-parent)\n\nI agree. Just recently I started missing that feature too at $DAYJOB.\n\n> 2. `git submodule summary` should have an option to display full logs\n> or \"first-parent\" logs.\n\nNo objection against that. Maybe now is a good time to make `git\nsubmodule summary` use `git diff --submodule` internally to make\nthem behave the same?\n\n> For #1, do you recommend adding a 3rd setting for `diff.submodule`\n> config? Something like \"full-log\" or something? Or an entirely new\n> config?\n\nI'd go with a 3rd setting for diff.submodule (and \"full-log\" would\nhave been my first choice too ;-).\n\n > I noticed that in diff.h, the DIFF_OPT flags already consume\n> 31 bits. If this is a 32-bit flag, there is only 1 bit left. If we go\n> with a 3rd setting for `diff.submodule` I think this might consume the\n> last bit.\n\nYup. But I'm not sure we can do anything about it.\n\n> We could also make `git diff --submodule` default to the \"full log\"\n> type, and if users want only first parent logs in submodule summary,\n> they'd have to execute `git submodule summary` instead.\n\nPlease do not change defaults that people lived fine with for years\nlightly. But I won't object changing that on a major version if a\nmajority of users request that.\n"},{"id":"260544","messageId":"CAHd499CRge9Y6VzdC_ngXS4WxuQ9HizXQJzLpX3iQStY5Cg=6g@mail.gmail.com","threadId":"39185","inReplyTo":"5547C961.7070909@web.de","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2015-05-04T20:21:31Z","receivedAt":"2015-05-04T20:21:31Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Mon, May 4, 2015 at 2:32 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:\n> Am 04.05.2015 um 17:05 schrieb Robert Dailey:\n>>\n>> On Fri, May 1, 2015 at 12:57 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n>>>\n>>> Hi,\n>>>\n>>> On Wed, Apr 29, 2015 at 03:53:11PM -0500, Robert Dailey wrote:\n>>>>\n>>>> I am attempting to diff a submodule modified in my working copy and\n>>>> the only difference is a merge commit. However, I do not get the\n>>>> \"full\" range of commits introduced by the merge commit when I diff it:\n>>>>\n>>>> $ git diff --submodule=log Core\n>>>> Submodule Core 8b4ec60..def2f3b:\n>>>>    > Merge remote-tracking branch 'origin/master-ah3k'\n>>>>\n>>>> However if I go inside my submodule and run `git log` by hand, I get\n>>>> more information about the TRUE commits introduced:\n>>>>\n>>>> $ git log --oneline 8b4ec60..def2f3b\n>>>> def2f3b Merge remote-tracking branch 'origin/master-ah3k'\n>>>> 015c961 Remove log spam in FontManager\n>>>> 7713ba1 Update third party submodule to latest\n>>>> 10aac78 Merge pull request #9 in FE/core from\n>>>> feature/FE-1348-selecting-continue-on-zero-balance to master-ah3k\n>>>> 287882f FE-1376 Nedd to remain in check detail screen when selecting\n>>>> donation after SBI\n>>>> a5a6bed Do not overwrite the current check# within loop\n>>>> dfb8547 Adding list of checks to CRspChecks before saving\n>>>> 1be280a FE-1354: Guest logged out in specific multiple check scenario\n>>>> de06d5a [FE-1348] Fix PATT exit while checks still open\n>>>>\n>>>> It's almost as if the `git diff --submodule=log` approach is passing\n>>>> in --first-parent to git log, which would exclude commits in the range\n>>>> that I'm seeing when I run git log manually.\n>>>\n>>>\n>>> That is exactly the case. In prepare_submodule_summary() that option is\n>>> set before doing the revision walk.\n>>>\n>>>> Is this by design? Is there a way to enable the full log history with\n>>>> `git diff` on a submodule?\n>>>\n>>>\n>>> This stems from the first implementation for showing submodule diffs in\n>>> commit 752c0c24. I guess this was done deliberately to limit the amount\n>>> of output you get for a submodule. At the moment this is hardcoded but I\n>>> think there is nothing wrong with adding another option to include the\n>>> full log.\n>>>\n>>> Cheers Heiko\n>>\n>>\n>> I will go ahead and work on this feature. Here is what I'd like to see:\n>>\n>> 1. `git diff --submodule` should have the ability to display full logs\n>> vs current logs (i.e. without --first-parent)\n>\n>\n> I agree. Just recently I started missing that feature too at $DAYJOB.\n>\n>> 2. `git submodule summary` should have an option to display full logs\n>> or \"first-parent\" logs.\n>\n>\n> No objection against that. Maybe now is a good time to make `git\n> submodule summary` use `git diff --submodule` internally to make\n> them behave the same?\n>\n>> For #1, do you recommend adding a 3rd setting for `diff.submodule`\n>> config? Something like \"full-log\" or something? Or an entirely new\n>> config?\n>\n>\n> I'd go with a 3rd setting for diff.submodule (and \"full-log\" would\n> have been my first choice too ;-).\n>\n>> I noticed that in diff.h, the DIFF_OPT flags already consume\n>>\n>> 31 bits. If this is a 32-bit flag, there is only 1 bit left. If we go\n>> with a 3rd setting for `diff.submodule` I think this might consume the\n>> last bit.\n>\n>\n> Yup. But I'm not sure we can do anything about it.\n>\n>> We could also make `git diff --submodule` default to the \"full log\"\n>> type, and if users want only first parent logs in submodule summary,\n>> they'd have to execute `git submodule summary` instead.\n>\n>\n> Please do not change defaults that people lived fine with for years\n> lightly. But I won't object changing that on a major version if a\n> majority of users request that.\n\nSince I am not a linux user, I have implemented this feature against\nthe Git for Windows fork of git. I am not able to verify changes if I\nmake them directly against the Git repository. Is it OK if you guys\nend up getting this as an upstream patch later from that project? Also\nI am not familiar with the bash unit tests, I will need help with\nthat.\n"},{"id":"260549","messageId":"20150504205104.GA28246@sandbox-ub1410","threadId":"39185","inReplyTo":"CAHd499CRge9Y6VzdC_ngXS4WxuQ9HizXQJzLpX3iQStY5Cg=6g@mail.gmail.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2015-05-04T20:51:04Z","receivedAt":"2015-05-04T20:51:04Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Mon, May 04, 2015 at 03:21:31PM -0500, Robert Dailey wrote:\n> Since I am not a linux user, I have implemented this feature against\n> the Git for Windows fork of git. I am not able to verify changes if I\n> make them directly against the Git repository. Is it OK if you guys\n> end up getting this as an upstream patch later from that project? Also\n> I am not familiar with the bash unit tests, I will need help with\n> that.\n\nI think there is nothing wrong with implementing it in the Windows\ndevelopment environment and then sending the patch directly here. As\nlong as it is not Windows specific (which it should not be) that should\nbe fine and you save the Windows guys the work to get the patch\nupstream (because here is upstream not there ;-)).\n\nHave a look at some tests, they are quite simple. Basically they run git\ncommands in a && chain and the resulting return code tells the testsuite\nwhether that test succeeded or not. Maybe have a look at the test for\nthe existing --submodule option (t4041-diff-submodule-option.sh) as an\nexample. You can probably reuse the complete setup there and just add a\nnew test for the new option with the expected output. There is also a\nREADME in the t/ folder. HTH.\n\nCheers Heiko\n"},{"id":"260551","messageId":"xmqqegmwrr6k.fsf@gitster.dls.corp.google.com","threadId":"39185","inReplyTo":"CAHd499B=EcgYiTMFt9VYhj45bRkP8h9TBk1B0cr8fYFuXNe_mQ@mail.gmail.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-04T21:03:31Z","receivedAt":"2015-05-04T21:03:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robert Dailey <rcdailey.lists@gmail.com> writes:\n\n> For #1, do you recommend adding a 3rd setting for `diff.submodule`\n> config? Something like \"full-log\" or something? Or an entirely new\n> config? I noticed that in diff.h, the DIFF_OPT flags already consume\n> 31 bits. If this is a 32-bit flag, there is only 1 bit left. If we go\n> with a 3rd setting for `diff.submodule` I think this might consume the\n> last bit.\n\nUnless you are using opts->touched_flags infrastructure to do funky\ndefaulting dance, there is really nothing that forces you to use a\nbit from the flag word for your new feature.  diff_options have many\nstandalone int fields that are used for either boolean or a small\nenum (e.g. degraded_cc_to_c) and it is perfectly fine to use a new\nsuch field.\n"},{"id":"260582","messageId":"37f399418bbebb3b53a50bf8daffcdc0@www.dscho.org","threadId":"39185","inReplyTo":"CAHd499CRge9Y6VzdC_ngXS4WxuQ9HizXQJzLpX3iQStY5Cg=6g@mail.gmail.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-05T05:49:54Z","receivedAt":"2015-05-05T05:49:54Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Robert,\n\nOn 2015-05-04 22:21, Robert Dailey wrote:\n\n> Since I am not a linux user, I have implemented this feature against\n> the Git for Windows fork of git. I am not able to verify changes if I\n> make them directly against the Git repository.\n\nThat is why I worked so hard on support of Vagrant: https://github.com/msysgit/msysgit/wiki/Vagrant -- in short, it makes it dead easy for you to set up a *minimal* Linux VM inside your Git SDK and interact with it via ssh.\n\nWith the Vagrant solution, you can easily test Linux Git even on Windows.\n\nCiao,\nJohannes\n"},{"id":"261372","messageId":"CAHd499Do2aB5E_=aDzkoDssEbgz181rH36X28Oe7Zcok2f=zBQ@mail.gmail.com","threadId":"39185","inReplyTo":"37f399418bbebb3b53a50bf8daffcdc0@www.dscho.org","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2015-05-15T20:33:07Z","receivedAt":"2015-05-15T20:33:07Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Tue, May 5, 2015 at 12:49 AM, Johannes Schindelin\n<johannes.schindelin@gmx.de> wrote:\n> Hi Robert,\n>\n> On 2015-05-04 22:21, Robert Dailey wrote:\n>\n>> Since I am not a linux user, I have implemented this feature against\n>> the Git for Windows fork of git. I am not able to verify changes if I\n>> make them directly against the Git repository.\n>\n> That is why I worked so hard on support of Vagrant: https://github.com/msysgit/msysgit/wiki/Vagrant -- in short, it makes it dead easy for you to set up a *minimal* Linux VM inside your Git SDK and interact with it via ssh.\n>\n> With the Vagrant solution, you can easily test Linux Git even on Windows.\n>\n> Ciao,\n> Johannes\n\nAt the moment I have a \"half-ass\" patch attached. This implements the\nfeature itself. I'm able to test this and it seems to be working.\nPlease note I'm a C++ developer and straight C / Bash are not my\nstrong suits. I apologize in advance for any mistakes. I am open to\ntaking recommendations for corrections.\n\nI'm not sure how I can verify the feature in a unit test. In addition\nto learning bash scripting well enough to write the test, I am not\nsure how to use git to check for the additional commits. Plus the repo\nfor the test will need to handle a submodule change to a merge commit\nas well. Any advice on setting up a good test case for this? What\nconditions should I check for, as far as log output goes?\n\n\nFrom 66db185ab76d0d893792d03dc0fc4913d868f218 Mon Sep 17 00:00:00 2001\nFrom: Robert Dailey <rcdailey@gmail.com>\nDate: Tue, 5 May 2015 21:56:26 +0100\nSubject: [PATCH] Add 'full-log' option to diff.submodule\n\nLike the 'log' option to `diff --submodule`, 'full-log' provides\nlogs without the `--first-parent` option.\n---\n diff.c      | 16 ++++++++++++----\n diff.h      |  1 +\n submodule.c |  9 +++++----\n submodule.h |  3 ++-\n 4 files changed, 20 insertions(+), 9 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 7500c55..58c4872 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -128,10 +128,18 @@ static int parse_dirstat_params(struct diff_options *options, const char *params\n \n static int parse_submodule_params(struct diff_options *options, const char *value)\n {\n-\tif (!strcmp(value, \"log\"))\n+\tif (!strcmp(value, \"log\")) {\n \t\tDIFF_OPT_SET(options, SUBMODULE_LOG);\n-\telse if (!strcmp(value, \"short\"))\n+\t\tDIFF_OPT_CLR(options, SUBMODULE_FULL_LOG);\n+\t}\n+\telse if (!strcmp(value, \"full-log\")) {\n+\t\tDIFF_OPT_SET(options, SUBMODULE_FULL_LOG);\n+\t\tDIFF_OPT_CLR(options, SUBMODULE_LOG);\n+\t}\n+\telse if (!strcmp(value, \"short\")) {\n \t\tDIFF_OPT_CLR(options, SUBMODULE_LOG);\n+\t\tDIFF_OPT_CLR(options, SUBMODULE_FULL_LOG);\n+\t}\n \telse\n \t\treturn -1;\n \treturn 0;\n@@ -2240,7 +2248,7 @@ static void builtin_diff(const char *name_a,\n \tstruct strbuf header = STRBUF_INIT;\n \tconst char *line_prefix = diff_line_prefix(o);\n \n-\tif (DIFF_OPT_TST(o, SUBMODULE_LOG) &&\n+\tif ((DIFF_OPT_TST(o, SUBMODULE_LOG) || DIFF_OPT_TST(o, SUBMODULE_FULL_LOG)) &&\n \t\t\t(!one->mode || S_ISGITLINK(one->mode)) &&\n \t\t\t(!two->mode || S_ISGITLINK(two->mode))) {\n \t\tconst char *del = diff_get_color_opt(o, DIFF_FILE_OLD);\n@@ -2248,7 +2256,7 @@ static void builtin_diff(const char *name_a,\n \t\tshow_submodule_summary(o->file, one->path ? one->path : two->path,\n \t\t\t\tline_prefix,\n \t\t\t\tone->sha1, two->sha1, two->dirty_submodule,\n-\t\t\t\tmeta, del, add, reset);\n+\t\t\t\tmeta, del, add, reset, DIFF_OPT_TST(o, SUBMODULE_FULL_LOG));\n \t\treturn;\n \t}\n \ndiff --git a/diff.h b/diff.h\nindex b4a624d..95f319c 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -90,6 +90,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_DIRSTAT_BY_LINE     (1 << 28)\n #define DIFF_OPT_FUNCCONTEXT         (1 << 29)\n #define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)\n+#define DIFF_OPT_SUBMODULE_FULL_LOG  (1 << 31)\n \n #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)\n #define DIFF_OPT_TOUCHED(opts, flag)    ((opts)->touched_flags & DIFF_OPT_##flag)\ndiff --git a/submodule.c b/submodule.c\nindex d37d400..f98173e 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -290,14 +290,14 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,\n \n static int prepare_submodule_summary(struct rev_info *rev, const char *path,\n \t\tstruct commit *left, struct commit *right,\n-\t\tint *fast_forward, int *fast_backward)\n+\t\tint *fast_forward, int *fast_backward, unsigned full_log)\n {\n \tstruct commit_list *merge_bases, *list;\n \n \tinit_revisions(rev, NULL);\n \tsetup_revisions(0, NULL, rev, NULL);\n \trev->left_right = 1;\n-\trev->first_parent_only = 1;\n+\trev->first_parent_only = full_log ? 0 : 1;\n \tleft->object.flags |= SYMMETRIC_LEFT;\n \tadd_pending_object(rev, &left->object, path);\n \tadd_pending_object(rev, &right->object, path);\n@@ -363,7 +363,8 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\tconst char *line_prefix,\n \t\tunsigned char one[20], unsigned char two[20],\n \t\tunsigned dirty_submodule, const char *meta,\n-\t\tconst char *del, const char *add, const char *reset)\n+\t\tconst char *del, const char *add, const char *reset,\n+\t\tunsigned full_log)\n {\n \tstruct rev_info rev;\n \tstruct commit *left = NULL, *right = NULL;\n@@ -381,7 +382,7 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\t !(right = lookup_commit_reference(two)))\n \t\tmessage = \"(commits not present)\";\n \telse if (prepare_submodule_summary(&rev, path, left, right,\n-\t\t\t\t\t   &fast_forward, &fast_backward))\n+\t\t\t\t\t   &fast_forward, &fast_backward, full_log))\n \t\tmessage = \"(revision walker failed)\";\n \n \tif (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)\ndiff --git a/submodule.h b/submodule.h\nindex 7beec48..301358b 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -26,7 +26,8 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\tconst char *line_prefix,\n \t\tunsigned char one[20], unsigned char two[20],\n \t\tunsigned dirty_submodule, const char *meta,\n-\t\tconst char *del, const char *add, const char *reset);\n+\t\tconst char *del, const char *add, const char *reset,\n+\t\tunsigned full_log);\n void set_config_fetch_recurse_submodules(int value);\n void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n int fetch_populated_submodules(const struct argv_array *options,\n-- \n2.4.1.windows.1\n\n"},{"id":"261423","messageId":"20150518123036.GB16841@book.hvoigt.net","threadId":"39185","inReplyTo":"CAHd499Do2aB5E_=aDzkoDssEbgz181rH36X28Oe7Zcok2f=zBQ@mail.gmail.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2015-05-18T12:30:36Z","receivedAt":"2015-05-18T12:30:36Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Fri, May 15, 2015 at 03:33:07PM -0500, Robert Dailey wrote:\n> On Tue, May 5, 2015 at 12:49 AM, Johannes Schindelin\n> <johannes.schindelin@gmx.de> wrote:\n> > Hi Robert,\n> >\n> > On 2015-05-04 22:21, Robert Dailey wrote:\n> >\n> >> Since I am not a linux user, I have implemented this feature against\n> >> the Git for Windows fork of git. I am not able to verify changes if I\n> >> make them directly against the Git repository.\n> >\n> > That is why I worked so hard on support of Vagrant: https://github.com/msysgit/msysgit/wiki/Vagrant -- in short, it makes it dead easy for you to set up a *minimal* Linux VM inside your Git SDK and interact with it via ssh.\n> >\n> > With the Vagrant solution, you can easily test Linux Git even on Windows.\n> >\n> > Ciao,\n> > Johannes\n> \n> At the moment I have a \"half-ass\" patch attached. This implements the\n> feature itself. I'm able to test this and it seems to be working.\n> Please note I'm a C++ developer and straight C / Bash are not my\n> strong suits. I apologize in advance for any mistakes. I am open to\n> taking recommendations for corrections.\n\nPlease inline the patch, so people can easily comment. Have a look at\nDocumentation/SubmittingPatches and patches on this list for an example.\nI have inlined your patch below for comments.\n\n> I'm not sure how I can verify the feature in a unit test. In addition\n> to learning bash scripting well enough to write the test, I am not\n> sure how to use git to check for the additional commits. Plus the repo\n> for the test will need to handle a submodule change to a merge commit\n> as well. Any advice on setting up a good test case for this? What\n> conditions should I check for, as far as log output goes?\n\nThe testsuite can be found in t/ the README there describes most of it.\nHave a look at t4041-diff-submodule-option.sh and imitate the tests for\nthe existing log option. What they basically do is: Write a file with\nthe expected output of the diff and then compare the actual output with\nit. That should also be possible for your option.\n\nAs for the merge commit: If there is no merge commit in the submodule\nthat is used for testing you can simply add a sequence of git commands\nthat manufactures the situation in the test repository as you need it.\n\n'test_pause' is a helpful command to interactively debug/develop tests.\nRun the test with the -v -i switches (maybe -d) when developing.\n\nComments for your patch please see below.\n\nCheers Heiko\n\n> From: Robert Dailey <rcdailey@gmail.com>\n> Subject: [PATCH] Add 'full-log' option to diff.submodule\n> \n> Like the 'log' option to `diff --submodule`, 'full-log' provides\n> logs without the `--first-parent` option.\n> ---\n>  diff.c      | 16 ++++++++++++----\n>  diff.h      |  1 +\n>  submodule.c |  9 +++++----\n>  submodule.h |  3 ++-\n>  4 files changed, 20 insertions(+), 9 deletions(-)\n> \n> diff --git a/diff.c b/diff.c\n> index 7500c55..58c4872 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -128,10 +128,18 @@ static int parse_dirstat_params(struct diff_options *options, const char *params\n>  \n>  static int parse_submodule_params(struct diff_options *options, const char *value)\n>  {\n> -\tif (!strcmp(value, \"log\"))\n> +\tif (!strcmp(value, \"log\")) {\n>  \t\tDIFF_OPT_SET(options, SUBMODULE_LOG);\n> -\telse if (!strcmp(value, \"short\"))\n> +\t\tDIFF_OPT_CLR(options, SUBMODULE_FULL_LOG);\n> +\t}\n> +\telse if (!strcmp(value, \"full-log\")) {\n> +\t\tDIFF_OPT_SET(options, SUBMODULE_FULL_LOG);\n> +\t\tDIFF_OPT_CLR(options, SUBMODULE_LOG);\n> +\t}\n> +\telse if (!strcmp(value, \"short\")) {\n>  \t\tDIFF_OPT_CLR(options, SUBMODULE_LOG);\n> +\t\tDIFF_OPT_CLR(options, SUBMODULE_FULL_LOG);\n> +\t}\n\nHere I think clearing the bits first and then setting them would be\nsimpler and less error prone for further extensions. E.g. in the\nbeginning of the function:\n\n\tDIFF_OPT_CLR(options, SUBMODULE_LOG);\n\tDIFF_OPT_CLR(options, SUBMODULE_FULL_LOG);\n\nand then\n\n\tif (!strcmp(value, \"log\"))\n\t\tDIFF_OPT_SET(options, SUBMODULE_LOG);\n\telse if (...\n\n\n>  \telse\n>  \t\treturn -1;\n>  \treturn 0;\n> @@ -2240,7 +2248,7 @@ static void builtin_diff(const char *name_a,\n>  \tstruct strbuf header = STRBUF_INIT;\n>  \tconst char *line_prefix = diff_line_prefix(o);\n>  \n> -\tif (DIFF_OPT_TST(o, SUBMODULE_LOG) &&\n> +\tif ((DIFF_OPT_TST(o, SUBMODULE_LOG) || DIFF_OPT_TST(o, SUBMODULE_FULL_LOG)) &&\n\nTry to keep your line length less than 80 characters.\n(Documentation/CodingGuidelines)\n\n>  \t\t\t(!one->mode || S_ISGITLINK(one->mode)) &&\n>  \t\t\t(!two->mode || S_ISGITLINK(two->mode))) {\n>  \t\tconst char *del = diff_get_color_opt(o, DIFF_FILE_OLD);\n> @@ -2248,7 +2256,7 @@ static void builtin_diff(const char *name_a,\n>  \t\tshow_submodule_summary(o->file, one->path ? one->path : two->path,\n>  \t\t\t\tline_prefix,\n>  \t\t\t\tone->sha1, two->sha1, two->dirty_submodule,\n> -\t\t\t\tmeta, del, add, reset);\n> +\t\t\t\tmeta, del, add, reset, DIFF_OPT_TST(o, SUBMODULE_FULL_LOG));\n\nSame as above.\n\n>  \t\treturn;\n>  \t}\n>  \n> diff --git a/diff.h b/diff.h\n> index b4a624d..95f319c 100644\n> --- a/diff.h\n> +++ b/diff.h\n> @@ -90,6 +90,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n>  #define DIFF_OPT_DIRSTAT_BY_LINE     (1 << 28)\n>  #define DIFF_OPT_FUNCCONTEXT         (1 << 29)\n>  #define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)\n> +#define DIFF_OPT_SUBMODULE_FULL_LOG  (1 << 31)\n>  \n>  #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)\n>  #define DIFF_OPT_TOUCHED(opts, flag)    ((opts)->touched_flags & DIFF_OPT_##flag)\n> diff --git a/submodule.c b/submodule.c\n> index d37d400..f98173e 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -290,14 +290,14 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,\n>  \n>  static int prepare_submodule_summary(struct rev_info *rev, const char *path,\n>  \t\tstruct commit *left, struct commit *right,\n> -\t\tint *fast_forward, int *fast_backward)\n> +\t\tint *fast_forward, int *fast_backward, unsigned full_log)\n>  {\n>  \tstruct commit_list *merge_bases, *list;\n>  \n>  \tinit_revisions(rev, NULL);\n>  \tsetup_revisions(0, NULL, rev, NULL);\n>  \trev->left_right = 1;\n> -\trev->first_parent_only = 1;\n> +\trev->first_parent_only = full_log ? 0 : 1;\n>  \tleft->object.flags |= SYMMETRIC_LEFT;\n>  \tadd_pending_object(rev, &left->object, path);\n>  \tadd_pending_object(rev, &right->object, path);\n> @@ -363,7 +363,8 @@ void show_submodule_summary(FILE *f, const char *path,\n>  \t\tconst char *line_prefix,\n>  \t\tunsigned char one[20], unsigned char two[20],\n>  \t\tunsigned dirty_submodule, const char *meta,\n> -\t\tconst char *del, const char *add, const char *reset)\n> +\t\tconst char *del, const char *add, const char *reset,\n> +\t\tunsigned full_log)\n>  {\n>  \tstruct rev_info rev;\n>  \tstruct commit *left = NULL, *right = NULL;\n> @@ -381,7 +382,7 @@ void show_submodule_summary(FILE *f, const char *path,\n>  \t\t !(right = lookup_commit_reference(two)))\n>  \t\tmessage = \"(commits not present)\";\n>  \telse if (prepare_submodule_summary(&rev, path, left, right,\n> -\t\t\t\t\t   &fast_forward, &fast_backward))\n> +\t\t\t\t\t   &fast_forward, &fast_backward, full_log))\n\nLine length.\n\n>  \t\tmessage = \"(revision walker failed)\";\n>  \n>  \tif (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)\n> diff --git a/submodule.h b/submodule.h\n> index 7beec48..301358b 100644\n> --- a/submodule.h\n> +++ b/submodule.h\n> @@ -26,7 +26,8 @@ void show_submodule_summary(FILE *f, const char *path,\n>  \t\tconst char *line_prefix,\n>  \t\tunsigned char one[20], unsigned char two[20],\n>  \t\tunsigned dirty_submodule, const char *meta,\n> -\t\tconst char *del, const char *add, const char *reset);\n> +\t\tconst char *del, const char *add, const char *reset,\n> +\t\tunsigned full_log);\n>  void set_config_fetch_recurse_submodules(int value);\n>  void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n>  int fetch_populated_submodules(const struct argv_array *options,\n\nApart from the comments above, your patch looks good to me.\n\nCheers Heiko\n"},{"id":"261451","messageId":"CAHd499CETM2jmZ2iJk=AoXtjLUCQ==u6q9Z5P-3EVGSY48FY_A@mail.gmail.com","threadId":"39185","inReplyTo":"20150518123036.GB16841@book.hvoigt.net","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2015-05-18T15:06:32Z","receivedAt":"2015-05-18T15:06:32Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Mon, May 18, 2015 at 7:30 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> Hi,\n>\n> On Fri, May 15, 2015 at 03:33:07PM -0500, Robert Dailey wrote:\n>> On Tue, May 5, 2015 at 12:49 AM, Johannes Schindelin\n>> <johannes.schindelin@gmx.de> wrote:\n>> > Hi Robert,\n>> >\n>> > On 2015-05-04 22:21, Robert Dailey wrote:\n>> >\n>> >> Since I am not a linux user, I have implemented this feature against\n>> >> the Git for Windows fork of git. I am not able to verify changes if I\n>> >> make them directly against the Git repository.\n>> >\n>> > That is why I worked so hard on support of Vagrant: https://github.com/msysgit/msysgit/wiki/Vagrant -- in short, it makes it dead easy for you to set up a *minimal* Linux VM inside your Git SDK and interact with it via ssh.\n>> >\n>> > With the Vagrant solution, you can easily test Linux Git even on Windows.\n>> >\n>> > Ciao,\n>> > Johannes\n>>\n>> At the moment I have a \"half-ass\" patch attached. This implements the\n>> feature itself. I'm able to test this and it seems to be working.\n>> Please note I'm a C++ developer and straight C / Bash are not my\n>> strong suits. I apologize in advance for any mistakes. I am open to\n>> taking recommendations for corrections.\n>\n> Please inline the patch, so people can easily comment. Have a look at\n> Documentation/SubmittingPatches and patches on this list for an example.\n> I have inlined your patch below for comments.\n>\n>> I'm not sure how I can verify the feature in a unit test. In addition\n>> to learning bash scripting well enough to write the test, I am not\n>> sure how to use git to check for the additional commits. Plus the repo\n>> for the test will need to handle a submodule change to a merge commit\n>> as well. Any advice on setting up a good test case for this? What\n>> conditions should I check for, as far as log output goes?\n>\n> The testsuite can be found in t/ the README there describes most of it.\n> Have a look at t4041-diff-submodule-option.sh and imitate the tests for\n> the existing log option. What they basically do is: Write a file with\n> the expected output of the diff and then compare the actual output with\n> it. That should also be possible for your option.\n>\n> As for the merge commit: If there is no merge commit in the submodule\n> that is used for testing you can simply add a sequence of git commands\n> that manufactures the situation in the test repository as you need it.\n>\n> 'test_pause' is a helpful command to interactively debug/develop tests.\n> Run the test with the -v -i switches (maybe -d) when developing.\n>\n> Comments for your patch please see below.\n>\n> <snip>\n\nUnfortunately I find it unintuitive and counter productive to perform\ninline patches or do anything on a mailing list. Especially on\nWindows, it's a pain to setup git to effectively do this. Also I read\nmailing lists through Gmail which does not offer a proper monospace\nfont view or syntax coloring to effectively review patches and\ncomments pertaining to them.\n\nSince I am not willing to properly follow your process, I will\nwithdraw my patch. However it is here if someone else wishes to take\nit over. Really wish you guys used github's amazing features but I\nunderstand that Linus has already made his decision in that matter.\n\nI'm sorry I couldn't be more agreeable on the matter. Thanks for the\ntime you spent reviewing my patch.\n"},{"id":"261543","messageId":"20150519104413.GA17458@book.hvoigt.net","threadId":"39185","inReplyTo":"CAHd499CETM2jmZ2iJk=AoXtjLUCQ==u6q9Z5P-3EVGSY48FY_A@mail.gmail.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2015-05-19T10:44:18Z","receivedAt":"2015-05-19T10:44:18Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Mon, May 18, 2015 at 10:06:32AM -0500, Robert Dailey wrote:\n> Unfortunately I find it unintuitive and counter productive to perform\n> inline patches or do anything on a mailing list. Especially on\n> Windows, it's a pain to setup git to effectively do this. Also I read\n> mailing lists through Gmail which does not offer a proper monospace\n> font view or syntax coloring to effectively review patches and\n> comments pertaining to them.\n\nAre you sure you are not overestimating the effort it takes to send\npatches inline? Once you've got your user agent correctly setup its just\na matter of copy and paste instead of attaching the patch. On Windows I\nwould probably use Thunderbird which has a section in the format-patch\ndocumentation how to configure it. Compared to the effort you probably\nspent on writing your patch isn't this bit of extra effort neglectable?\nAnd your patch is almost done. It just needs some tests and maybe a few\nrounds on the mailinglist after that.\n\n> Since I am not willing to properly follow your process, I will\n> withdraw my patch. However it is here if someone else wishes to take\n> it over. Really wish you guys used github's amazing features but I\n> understand that Linus has already made his decision in that matter.\n\nIt not just Linus decision it is also a matter of many people are used\nto this workflow. AFAIR there have been many discussions and tries about\nusing other tools. Email has many advantages which a webinterface does\nnot provide. It is simply less effort that one person adjusts to this\nworkflow instead of changing many peoples working workflow.\n\n> I'm sorry I couldn't be more agreeable on the matter. Thanks for the\n> time you spent reviewing my patch.\n\nIf you are really this fixed in your workflow that would be too bad.\n\nCheers Heiko\n"},{"id":"261585","messageId":"CAHd499D9vOtLOBj2s5EOfsojSStZY+HdZR35icZ5cssLNkcD-A@mail.gmail.com","threadId":"39185","inReplyTo":"20150519104413.GA17458@book.hvoigt.net","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2015-05-19T19:29:55Z","receivedAt":"2015-05-19T19:29:55Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Tue, May 19, 2015 at 5:44 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> On Mon, May 18, 2015 at 10:06:32AM -0500, Robert Dailey wrote:\n>> Unfortunately I find it unintuitive and counter productive to perform\n>> inline patches or do anything on a mailing list. Especially on\n>> Windows, it's a pain to setup git to effectively do this. Also I read\n>> mailing lists through Gmail which does not offer a proper monospace\n>> font view or syntax coloring to effectively review patches and\n>> comments pertaining to them.\n>\n> Are you sure you are not overestimating the effort it takes to send\n> patches inline? Once you've got your user agent correctly setup its just\n> a matter of copy and paste instead of attaching the patch. On Windows I\n> would probably use Thunderbird which has a section in the format-patch\n> documentation how to configure it. Compared to the effort you probably\n> spent on writing your patch isn't this bit of extra effort neglectable?\n> And your patch is almost done. It just needs some tests and maybe a few\n> rounds on the mailinglist after that.\n>\n>> Since I am not willing to properly follow your process, I will\n>> withdraw my patch. However it is here if someone else wishes to take\n>> it over. Really wish you guys used github's amazing features but I\n>> understand that Linus has already made his decision in that matter.\n>\n> It not just Linus decision it is also a matter of many people are used\n> to this workflow. AFAIR there have been many discussions and tries about\n> using other tools. Email has many advantages which a webinterface does\n> not provide. It is simply less effort that one person adjusts to this\n> workflow instead of changing many peoples working workflow.\n>\n>> I'm sorry I couldn't be more agreeable on the matter. Thanks for the\n>> time you spent reviewing my patch.\n>\n> If you are really this fixed in your workflow that would be too bad.\n\nHow do you send your patches inline? Do you use git send-email? I have\ntried that and it is horrible to setup. Do you just copy/paste the\npatch inline in your compose window?\n\nIt would be much simpler to fork Git, create a branch, make my change,\nand initiate a pull request. I can get email notifications on comments\nto my PR diff and address them with subsequent pushes to my branch\n(which would also automatically update the code review). Turn around\ntimes for collaborating on a change are much quicker via Github pull\nrequests.\n\nI am willing to review the typical workflow for contributing via git\non mailing lists but I haven't seen any informative reading material\non this. I just find using command line to email patches and dealing\nwith other issues not worth the trouble. Lack of syntax highlighting,\nlack of monospace font, the fact that I'm basically forced to install\nmail client software just to contribute a single git patch.\n"},{"id":"261592","messageId":"CAGZ79kZf_C=QkfQ+LV9G9fLsfqE9USrwkMiNdLmz0-BtPuAmRw@mail.gmail.com","threadId":"39185","inReplyTo":"CAHd499D9vOtLOBj2s5EOfsojSStZY+HdZR35icZ5cssLNkcD-A@mail.gmail.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-05-19T20:34:58Z","receivedAt":"2015-05-19T20:34:58Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, May 19, 2015 at 12:29 PM, Robert Dailey\n<rcdailey.lists@gmail.com> wrote:\n> How do you send your patches inline?\n\nThere are various ways to do so.\nIf you look at https://github.com/git/git/blob/master/Documentation/SubmittingPatches\nand search for Thunderbird (I used to use Thunderbird for a long time\nbefore switching to\ngit send-email, so I'll take that as an example) at the bottom:\n\n    Thunderbird, KMail, GMail\n    -------------------------\n\n    See the MUA-SPECIFIC HINTS section of git-format-patch(1).\n\nOk, indirection is the fun part of computers. ;)\nSo you'd look at the man page of git format patch,\nsuch as here http://git-scm.com/docs/git-format-patch\nand scroll the way down to MUA-SPECIFIC HINTS, which offers\n3 different ways of doing it. (decisions!)\n\n> Do you use git send-email?\n\nI do, but I remember my initial struggle with it (I will contribute only\none patch anyway, so why care?)\n\n> I have\n> tried that and it is horrible to setup. Do you just copy/paste the\n> patch inline in your compose window?\n\nOnce setup correctly git formatpatch / send-email are actually very\nconvenient (e.g. git send-email HEAD^ --to=git@vger.kernel.org will\njust work. And I have strong confidence in it continuing to work,\neven when Git decides to revamp the preferred patch format,\nline wrapping or other exotic stuff)\n\n>\n> It would be much simpler to fork Git, create a branch, make my change,\n> and initiate a pull request. I can get email notifications on comments\n> to my PR diff and address them with subsequent pushes to my branch\n> (which would also automatically update the code review). Turn around\n> times for collaborating on a change are much quicker via Github pull\n> requests.\n\nGithub has indeed an excellent product, even free for open source.\n\nThis workflow discussion was a topic at the GitMerge2015 conference,\nand there are essentially 2 groups, those who know how to send email\nand those who complain about it. A solution was agreed on by nearly all\nof the contributors. It would be awesome to have a git-to-email proxy,\nsuch that you could do a git push <proxy> master:refs/for/mailinglist\nand this proxy would convert the push into sending patch series to the\nmailing list. It could even convert the following discussion back into\ncomments (on Github?) but as a first step we'd want to try out a one\nway proxy.\n\nUnfortunately nobody stepped up to actually do the work, yet :(\n\n>\n> I am willing to review the typical workflow for contributing via git\n> on mailing lists but I haven't seen any informative reading material\n> on this. I just find using command line to email patches and dealing\n> with other issues not worth the trouble. Lack of syntax highlighting,\n> lack of monospace font, the fact that I'm basically forced to install\n> mail client software just to contribute a single git patch.\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"261775","messageId":"20150521125122.GA22553@book.hvoigt.net","threadId":"39185","inReplyTo":"CAHd499D9vOtLOBj2s5EOfsojSStZY+HdZR35icZ5cssLNkcD-A@mail.gmail.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2015-05-21T12:51:22Z","receivedAt":"2015-05-21T12:51:22Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Tue, May 19, 2015 at 02:29:55PM -0500, Robert Dailey wrote:\n> On Tue, May 19, 2015 at 5:44 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> > On Mon, May 18, 2015 at 10:06:32AM -0500, Robert Dailey wrote:\n> >> Unfortunately I find it unintuitive and counter productive to perform\n> >> inline patches or do anything on a mailing list. Especially on\n> >> Windows, it's a pain to setup git to effectively do this. Also I read\n> >> mailing lists through Gmail which does not offer a proper monospace\n> >> font view or syntax coloring to effectively review patches and\n> >> comments pertaining to them.\n> >\n> > Are you sure you are not overestimating the effort it takes to send\n> > patches inline? Once you've got your user agent correctly setup its just\n> > a matter of copy and paste instead of attaching the patch. On Windows I\n> > would probably use Thunderbird which has a section in the format-patch\n> > documentation how to configure it. Compared to the effort you probably\n> > spent on writing your patch isn't this bit of extra effort neglectable?\n> > And your patch is almost done. It just needs some tests and maybe a few\n> > rounds on the mailinglist after that.\n> >\n> >> Since I am not willing to properly follow your process, I will\n> >> withdraw my patch. However it is here if someone else wishes to take\n> >> it over. Really wish you guys used github's amazing features but I\n> >> understand that Linus has already made his decision in that matter.\n> >\n> > It not just Linus decision it is also a matter of many people are used\n> > to this workflow. AFAIR there have been many discussions and tries about\n> > using other tools. Email has many advantages which a webinterface does\n> > not provide. It is simply less effort that one person adjusts to this\n> > workflow instead of changing many peoples working workflow.\n> >\n> >> I'm sorry I couldn't be more agreeable on the matter. Thanks for the\n> >> time you spent reviewing my patch.\n> >\n> > If you are really this fixed in your workflow that would be too bad.\n> \n> How do you send your patches inline? Do you use git send-email? I have\n> tried that and it is horrible to setup. Do you just copy/paste the\n> patch inline in your compose window?\n\nFor bigger patch series I did use send-email but currently I am back to\njust using the compose window from whatever email client I am using. On\nWindows that would be Thunderbird. But when possible I am not using\nWindows.\n\n> It would be much simpler to fork Git, create a branch, make my change,\n> and initiate a pull request. I can get email notifications on comments\n> to my PR diff and address them with subsequent pushes to my branch\n> (which would also automatically update the code review). Turn around\n> times for collaborating on a change are much quicker via Github pull\n> requests.\n\nI think that depends more on the collaborators than on the tool. When\nyou get quick replies the turnaround times with both workflows are\nquick.\n\nIt would be nice if there was a perfect solution for every project that\neveryone could use but unfortunately there is not so we sometimes have\nto adjust. But I think its more matter of what you are used to. If you\ndid not have a github account but email software setup you could\ncomplain about the fact that you need to register a github account, fork\ngit, setup that fork in your local repository, ... instead of just copy\nand paste your change into the compose window and then send it to a\nmailinglist.\n\n> I am willing to review the typical workflow for contributing via git\n> on mailing lists but I haven't seen any informative reading material\n> on this. I just find using command line to email patches and dealing\n> with other issues not worth the trouble. Lack of syntax highlighting,\n> lack of monospace font, the fact that I'm basically forced to install\n> mail client software just to contribute a single git patch.\n\nAs already mentioned by Stefan there is Documentation/SubmittingPatches\nin the Git repository that describes everything and also has a section\non how to do that with Thunderbird.\n\nI tend to not do much on the commandline on Windows since it basically\nsucks there. For sending patches you just need\n\n\tgit format-patch HEAD^\n\nand thats it.\n\nCheers Heiko\n"},{"id":"261892","messageId":"CAFY1edZhznDENput-3E=GRrHsWXcWd5Pr3nieOVShuJ8gMfCiQ@mail.gmail.com","threadId":"39185","inReplyTo":"CAGZ79kZf_C=QkfQ+LV9G9fLsfqE9USrwkMiNdLmz0-BtPuAmRw@mail.gmail.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Roberto Tyley","fromEmail":"roberto.tyley@gmail.com","sentAt":"2015-05-22T09:17:03Z","receivedAt":"2015-05-22T09:17:03Z","isPatch":false,"sender":{"key":"roberto.tyley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/52038?v=4"},"body":"On Tuesday, 19 May 2015, Stefan Beller <sbeller@google.com> wrote:\n> On Tue, May 19, 2015 at 12:29 PM, Robert Dailey\n> <rcdailey.lists@gmail.com> wrote:\n> > How do you send your patches inline?\n>\n> This workflow discussion was a topic at the GitMerge2015 conference,\n> and there are essentially 2 groups, those who know how to send email\n> and those who complain about it. A solution was agreed on by nearly all\n> of the contributors. It would be awesome to have a git-to-email proxy,\n> such that you could do a git push <proxy> master:refs/for/mailinglist\n> and this proxy would convert the push into sending patch series to the\n> mailing list. It could even convert the following discussion back into\n> comments (on Github?) but as a first step we'd want to try out a one\n> way proxy.\n>\n> Unfortunately nobody stepped up to actually do the work, yet :(\n\nI've replied to this on a separate announcement thread on the Git mailing\nlist here:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/269699\n\n...I've created a new tool called submitGit, which aims to help.\n\n> > I am willing to review the typical workflow for contributing via git\n> > on mailing lists but I haven't seen any informative reading material\n> > on this. I just find using command line to email patches and dealing\n> > with other issues not worth the trouble. Lack of syntax highlighting,\n> > lack of monospace font, the fact that I'm basically forced to install\n> > mail client software just to contribute a single git patch.\n\nI'd be interested to know what you think!\n\nRoberto\n"},{"id":"262460","messageId":"55691DE3.70200@gmail.com","threadId":"39185","inReplyTo":"20150521125122.GA22553@book.hvoigt.net","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2015-05-30T02:18:11Z","receivedAt":"2015-05-30T02:18:11Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On 5/21/2015 7:51 AM, Heiko Voigt wrote:\n> On Tue, May 19, 2015 at 02:29:55PM -0500, Robert Dailey wrote:\n>> On Tue, May 19, 2015 at 5:44 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n>>> On Mon, May 18, 2015 at 10:06:32AM -0500, Robert Dailey wrote:\n>>>> Unfortunately I find it unintuitive and counter productive to perform\n>>>> inline patches or do anything on a mailing list. Especially on\n>>>> Windows, it's a pain to setup git to effectively do this. Also I read\n>>>> mailing lists through Gmail which does not offer a proper monospace\n>>>> font view or syntax coloring to effectively review patches and\n>>>> comments pertaining to them.\n>>>\n>>> Are you sure you are not overestimating the effort it takes to send\n>>> patches inline? Once you've got your user agent correctly setup its just\n>>> a matter of copy and paste instead of attaching the patch. On Windows I\n>>> would probably use Thunderbird which has a section in the format-patch\n>>> documentation how to configure it. Compared to the effort you probably\n>>> spent on writing your patch isn't this bit of extra effort neglectable?\n>>> And your patch is almost done. It just needs some tests and maybe a few\n>>> rounds on the mailinglist after that.\n>>>\n>>>> Since I am not willing to properly follow your process, I will\n>>>> withdraw my patch. However it is here if someone else wishes to take\n>>>> it over. Really wish you guys used github's amazing features but I\n>>>> understand that Linus has already made his decision in that matter.\n>>>\n>>> It not just Linus decision it is also a matter of many people are used\n>>> to this workflow. AFAIR there have been many discussions and tries about\n>>> using other tools. Email has many advantages which a webinterface does\n>>> not provide. It is simply less effort that one person adjusts to this\n>>> workflow instead of changing many peoples working workflow.\n>>>\n>>>> I'm sorry I couldn't be more agreeable on the matter. Thanks for the\n>>>> time you spent reviewing my patch.\n>>>\n>>> If you are really this fixed in your workflow that would be too bad.\n>>\n>> How do you send your patches inline? Do you use git send-email? I have\n>> tried that and it is horrible to setup. Do you just copy/paste the\n>> patch inline in your compose window?\n>\n> For bigger patch series I did use send-email but currently I am back to\n> just using the compose window from whatever email client I am using. On\n> Windows that would be Thunderbird. But when possible I am not using\n> Windows.\n>\n>> It would be much simpler to fork Git, create a branch, make my change,\n>> and initiate a pull request. I can get email notifications on comments\n>> to my PR diff and address them with subsequent pushes to my branch\n>> (which would also automatically update the code review). Turn around\n>> times for collaborating on a change are much quicker via Github pull\n>> requests.\n>\n> I think that depends more on the collaborators than on the tool. When\n> you get quick replies the turnaround times with both workflows are\n> quick.\n>\n> It would be nice if there was a perfect solution for every project that\n> everyone could use but unfortunately there is not so we sometimes have\n> to adjust. But I think its more matter of what you are used to. If you\n> did not have a github account but email software setup you could\n> complain about the fact that you need to register a github account, fork\n> git, setup that fork in your local repository, ... instead of just copy\n> and paste your change into the compose window and then send it to a\n> mailinglist.\n>\n>> I am willing to review the typical workflow for contributing via git\n>> on mailing lists but I haven't seen any informative reading material\n>> on this. I just find using command line to email patches and dealing\n>> with other issues not worth the trouble. Lack of syntax highlighting,\n>> lack of monospace font, the fact that I'm basically forced to install\n>> mail client software just to contribute a single git patch.\n>\n> As already mentioned by Stefan there is Documentation/SubmittingPatches\n> in the Git repository that describes everything and also has a section\n> on how to do that with Thunderbird.\n>\n> I tend to not do much on the commandline on Windows since it basically\n> sucks there. For sending patches you just need\n>\n> \tgit format-patch HEAD^\n>\n> and thats it.\n>\n> Cheers Heiko\n>\n\nSo I am working on trying to setup my environment (VM through Virtual \nBox) to do some testing on this. You all have encouraged me to try the \nmailing list review model. So I won't give up yet.\n\nIn the meantime I'd like to ask, do we even need to add an option for \nthis? What if we just make `diff.submodule log` not use --first-parent? \nThis seems like a backward compatible change in of itself. And it's \nsimpler to implement. I can't think of a good justification to add more \nsettings to an already hugely complex configuration scheme for such a \nminor difference in behavior.\n\nThoughts?\n"},{"id":"262464","messageId":"20150530104129.GA4309@book.hvoigt.net","threadId":"39185","inReplyTo":"55691DE3.70200@gmail.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2015-05-30T10:41:30Z","receivedAt":"2015-05-30T10:41:30Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Fri, May 29, 2015 at 09:18:11PM -0500, Robert Dailey wrote:\n> So I am working on trying to setup my environment (VM through Virtual Box)\n> to do some testing on this. You all have encouraged me to try the mailing\n> list review model. So I won't give up yet.\n\nI am not sure you need a VM or Linux environment. Of course it will be\nhelpful in case your tests do no pass on Linux (which they sometimes do\ndue to some differences between the OSes). But until we actually run\ninto that problem I do not see anything wrong developing your change\npurely on Windows. Since it seems you are more familiar with that\nplatform I would even encourage you to do so. That reduces the toolset\nfriction which you might experience in a new environment. Even if you\nrun into the problem, that your tests do not pass on Linux, we might be\nable to solve that on the list.\n\nHave you seen the github pull request -> mailing list proxy thing[1]? If\nthat helps you maybe you can test it and use it for your patch\nsubmission. I think nobody will be annoyed if we get some strange emails\non the list during that testing phase since that might help more\ncontributors to contribute.\n\n> In the meantime I'd like to ask, do we even need to add an option for this?\n> What if we just make `diff.submodule log` not use --first-parent? This seems\n> like a backward compatible change in of itself. And it's simpler to\n> implement. I can't think of a good justification to add more settings to an\n> already hugely complex configuration scheme for such a minor difference in\n> behavior.\n> \n> Thoughts?\n\nThis behavior has been with --first-parent for a long time. Even though\nit seems like a minor change in my experience there will be complaints\nfrom people that have got used to it and will now get big differences.\nYou never know how people use it. Since the extra value (full-log)\n(AFAIR) has reached a consensus on the list and it allows us to satisfy\nboth long log and short log users I would prefer to go that route.\n\nCheers Heiko\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/269699\n"},{"id":"262472","messageId":"xmqqbnh2ateo.fsf@gitster.dls.corp.google.com","threadId":"39185","inReplyTo":"55691DE3.70200@gmail.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-30T17:04:31Z","receivedAt":"2015-05-30T17:04:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robert Dailey <rcdailey.lists@gmail.com> writes:\n\n> In the meantime I'd like to ask, do we even need to add an option for\n> this? What if we just make `diff.submodule log` not use\n> --first-parent? This seems like a backward compatible change in of\n> itself.\n\nWhy?  People have relied on submodule-log not to include all the\nnoise coming from individual commits on side branches and instead\nappreciated seeing only the overview by merges of side branch topics\nbeing listed---why is regressing the system to inconvenience these\nexisting users \"a backward compatible change\"?\n\n> And it's simpler to implement. I can't think of a good\n> justification to add more settings to an already hugely complex\n> configuration scheme for such a minor difference in behavior.\n\nCareful, as that argument can cut both ways.  If it is so a minor\ndifference in behaviour, perhaps we can do without not just an\noption but a feature to omit --first-parent here.  That would be\neven simpler to implement, as you do not have to do anything.\n\nSo, if you think the new behaviour can help _some_ users, then you\nwould need the feature and a knob to enable it, I would think.\n"},{"id":"262484","messageId":"CAHd499B+icDskcsR7zxPfZ=8Nqwg14eb2h2LBuDx6=fnoO24AQ@mail.gmail.com","threadId":"39185","inReplyTo":"xmqqbnh2ateo.fsf@gitster.dls.corp.google.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2015-05-30T19:19:09Z","receivedAt":"2015-05-30T19:19:09Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Sat, May 30, 2015 at 12:04 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Robert Dailey <rcdailey.lists@gmail.com> writes:\n>\n>> In the meantime I'd like to ask, do we even need to add an option for\n>> this? What if we just make `diff.submodule log` not use\n>> --first-parent? This seems like a backward compatible change in of\n>> itself.\n>\n> Why?  People have relied on submodule-log not to include all the\n> noise coming from individual commits on side branches and instead\n> appreciated seeing only the overview by merges of side branch topics\n> being listed---why is regressing the system to inconvenience these\n> existing users \"a backward compatible change\"?\n\nBackward compatible in the sense that it does not break existing\nfunctionality. For example, removing -D from `git branch` would be a\nbackward breaking change.\n\nAlso not everyone has a disciplined merge-commit editing policy like\nthe Linux & Git projects do. In fact (especially in corporate\nenvironments), most merge commit messages are useless and\nauto-generated. It's in fact very common (depending on popular\ntooling) to not have the ability to edit these messages. Example:\n\n    Merge branch \"topic\" into \"master\"\n\nThis is the defacto message you get when you use Github, Atlassian\nStash, etc. any time you merge a PR. For repositories that only accept\nchanges through pull requests, it is not possible to generate a diff\nlog of submodules that is useful without showing the ancestor commits\non all parents. Right now 'log' literally gives you the following for\na submodule that has a mainline branch that does not accept direct\npushes (i.e. only PRs):\n\n    > Merge branch \"topic1\" into \"master\"\n    > Merge branch \"topic2\" into \"master\"\n    > Merge branch \"origin/develop\" into \"master\"\n    > Merge branch \"topic3\" into \"master\"\n\nI would argue that this situation is common -- more common than what\nyou would typically see in a well groomed git repository that does not\nuse PRs or always does rebase+ff-merge.\n\nThat is the basis for my concern. No functionality is broken and\ncators to the common case. But I guess since we're discussing 2\ndistinct workflows, it makes sense to have 2 options for viewing the\nsubmodule logs. Because if 'full-log' were indeed the default, it\nwould cause a lot of noise in the well-disciplined-merge-commit\nworkflow.\n\n>From a high level (to explain my motivation for my simplified and\narguably naive suggestion), I work with a lot of amateur users of Git.\nThey always complain about using Git and how \"SVN is better\". Yet they\ndo not accept the challenge of learning Git, which is a very\ncomplicated tool. Much of git I would argue is not very beginner (even\nuser) friendly (although things are certainly getting better). With\nsuch an advanced tool, with such complex configuration and behavior,\nand given all the friction it causes, I can only do my best to offer\nseemingly sensible alternatives to adding MORE configuration.\n\nAnyway it's just a thought. Glad to get feedback and I see both sides\nof the fence on this one.\n\n>> And it's simpler to implement. I can't think of a good\n>> justification to add more settings to an already hugely complex\n>> configuration scheme for such a minor difference in behavior.\n>\n> Careful, as that argument can cut both ways.  If it is so a minor\n> difference in behaviour, perhaps we can do without not just an\n> option but a feature to omit --first-parent here.  That would be\n> even simpler to implement, as you do not have to do anything.\n>\n> So, if you think the new behaviour can help _some_ users, then you\n> would need the feature and a knob to enable it, I would think.\n\nI don't really understand your contrasted example here. Can you explain:\n\n    \"...we can do without not just an option but a feature to omit\n--first-parent...\"\n\nAgain thanks for the feedback.\n"},{"id":"262485","messageId":"CAHd499BRhfbfGOWv217QT5GwLpOkY4B3j8t3MACGXz-ZdQbYtA@mail.gmail.com","threadId":"39185","inReplyTo":"CAHd499B+icDskcsR7zxPfZ=8Nqwg14eb2h2LBuDx6=fnoO24AQ@mail.gmail.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2015-05-30T19:37:39Z","receivedAt":"2015-05-30T19:37:39Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Sat, May 30, 2015 at 2:19 PM, Robert Dailey <rcdailey.lists@gmail.com> wrote:\n> On Sat, May 30, 2015 at 12:04 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Robert Dailey <rcdailey.lists@gmail.com> writes:\n>>\n>>> In the meantime I'd like to ask, do we even need to add an option for\n>>> this? What if we just make `diff.submodule log` not use\n>>> --first-parent? This seems like a backward compatible change in of\n>>> itself.\n>>\n>> Why?  People have relied on submodule-log not to include all the\n>> noise coming from individual commits on side branches and instead\n>> appreciated seeing only the overview by merges of side branch topics\n>> being listed---why is regressing the system to inconvenience these\n>> existing users \"a backward compatible change\"?\n>\n> Backward compatible in the sense that it does not break existing\n> functionality. For example, removing -D from `git branch` would be a\n> backward breaking change.\n>\n> Also not everyone has a disciplined merge-commit editing policy like\n> the Linux & Git projects do. In fact (especially in corporate\n> environments), most merge commit messages are useless and\n> auto-generated. It's in fact very common (depending on popular\n> tooling) to not have the ability to edit these messages. Example:\n>\n>     Merge branch \"topic\" into \"master\"\n>\n> This is the defacto message you get when you use Github, Atlassian\n> Stash, etc. any time you merge a PR. For repositories that only accept\n> changes through pull requests, it is not possible to generate a diff\n> log of submodules that is useful without showing the ancestor commits\n> on all parents. Right now 'log' literally gives you the following for\n> a submodule that has a mainline branch that does not accept direct\n> pushes (i.e. only PRs):\n>\n>     > Merge branch \"topic1\" into \"master\"\n>     > Merge branch \"topic2\" into \"master\"\n>     > Merge branch \"origin/develop\" into \"master\"\n>     > Merge branch \"topic3\" into \"master\"\n>\n> I would argue that this situation is common -- more common than what\n> you would typically see in a well groomed git repository that does not\n> use PRs or always does rebase+ff-merge.\n>\n> That is the basis for my concern. No functionality is broken and\n> cators to the common case. But I guess since we're discussing 2\n> distinct workflows, it makes sense to have 2 options for viewing the\n> submodule logs. Because if 'full-log' were indeed the default, it\n> would cause a lot of noise in the well-disciplined-merge-commit\n> workflow.\n>\n> From a high level (to explain my motivation for my simplified and\n> arguably naive suggestion), I work with a lot of amateur users of Git.\n> They always complain about using Git and how \"SVN is better\". Yet they\n> do not accept the challenge of learning Git, which is a very\n> complicated tool. Much of git I would argue is not very beginner (even\n> user) friendly (although things are certainly getting better). With\n> such an advanced tool, with such complex configuration and behavior,\n> and given all the friction it causes, I can only do my best to offer\n> seemingly sensible alternatives to adding MORE configuration.\n>\n> Anyway it's just a thought. Glad to get feedback and I see both sides\n> of the fence on this one.\n>\n>>> And it's simpler to implement. I can't think of a good\n>>> justification to add more settings to an already hugely complex\n>>> configuration scheme for such a minor difference in behavior.\n>>\n>> Careful, as that argument can cut both ways.  If it is so a minor\n>> difference in behaviour, perhaps we can do without not just an\n>> option but a feature to omit --first-parent here.  That would be\n>> even simpler to implement, as you do not have to do anything.\n>>\n>> So, if you think the new behaviour can help _some_ users, then you\n>> would need the feature and a knob to enable it, I would think.\n>\n> I don't really understand your contrasted example here. Can you explain:\n>\n>     \"...we can do without not just an option but a feature to omit\n> --first-parent...\"\n>\n> Again thanks for the feedback.\n\nI'm having some difficulty with the tests. What it looks like is that\nthe test repository is created by calling test-lib.sh at the top. Then\nby the time the commands after that are run, it's inside the temp\nrepository.\n\nAt the bottom of t4041, I add this:\n\n    test_create_repo sm3 &&\n    git add sm3 &&\n    cd sm3 &&\n    echo > foo.txt &&\n    git add foo.txt &&\n    git commit -m 'foo.txt' >/dev/null &&\n    git checkout -b topic >/dev/null &&\n    echo > topic.txt &&\n    git add topic.txt &&\n    git commit -m 'topic.txt' >/dev/null &&\n    git checkout master >/dev/null &&\n    git merge --no-ff --no-edit topic >/dev/null &&\n    cd .. &&\n    git diff --submodule=log sm3\n\nBut this does not work as I expect. How do I add a submodule and then\nsimulate creation of branches, commits, and merges, prior to running\ncode in \"text_expect_success\"?\n"},{"id":"262486","messageId":"xmqq382dc043.fsf@gitster.dls.corp.google.com","threadId":"39185","inReplyTo":"CAHd499B+icDskcsR7zxPfZ=8Nqwg14eb2h2LBuDx6=fnoO24AQ@mail.gmail.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-30T19:54:20Z","receivedAt":"2015-05-30T19:54:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robert Dailey <rcdailey.lists@gmail.com> writes:\n\n> On Sat, May 30, 2015 at 12:04 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Robert Dailey <rcdailey.lists@gmail.com> writes:\n>>\n>>> In the meantime I'd like to ask, do we even need to add an option for\n>>> this? What if we just make `diff.submodule log` not use\n>>> --first-parent? This seems like a backward compatible change in of\n>>> itself.\n>>\n>> Why?  People have relied on submodule-log not to include all the\n>> noise coming from individual commits on side branches and instead\n>> appreciated seeing only the overview by merges of side branch topics\n>> being listed---why is regressing the system to inconvenience these\n>> existing users \"a backward compatible change\"?\n>\n> Backward compatible in the sense that it does not break existing\n> functionality....\n\nAnd adding one-line-per-commit from side branches does break\nexisting functionality.  You seem to be arguing that more\ninformation is always good and does not break existing\nfunctionality, but summarizing by omitting irrelevant details *is* a\nfeature.  Do you honestly believe that a change to make the full\n\"log -p\" output in submodule log be a \"backward compatible\" change??\n\n>     > Merge branch \"topic1\" into \"master\"\n>     > Merge branch \"topic2\" into \"master\"\n>     > Merge branch \"origin/develop\" into \"master\"\n>     > Merge branch \"topic3\" into \"master\"\n\nIt is not a real argument; it is something you can fix by naming\nyour branches more sensibly, which would make you a better developer\nregardless of how submodule-log is shown.\n\n>>> And it's simpler to implement. I can't think of a good\n>>> justification to add more settings to an already hugely complex\n>>> configuration scheme for such a minor difference in behavior.\n>>\n>> Careful, as that argument can cut both ways.  If it is so a minor\n>> difference in behaviour, perhaps we can do without not just an\n>> option but a feature to omit --first-parent here.  That would be\n>> even simpler to implement, as you do not have to do anything.\n>\n> I don't really understand your contrasted example here. Can you explain:\n>\n>     \"...we can do without not just an option but a feature to omit\n> --first-parent...\"\n\nI am not sure how to phrase that any easier. Sorry.\n\nAssuming that you consider output with and without --first-parent\ndoes not make much of a difference (i.e. \"minor difference in\nbehaviour\"), instead of dropping --first-parent unconditionally,\nlike you seem to be pushing with the argument, we can\nunconditionally keep the --first-parent and do not change anything.\nThe end result would not make much of a difference either way, and\nnot doing anything is much simpler than dropping --first-parent.\n\nHopefully you can see how absurd that line of reasoning is.  So do\nnot make the same argument to push for changing the behaviour\nunconditionally.\n\nIf you think the new behaviour can help _some_ users, then you would\nneed the feature and a knob to enable it.\n"},{"id":"262487","messageId":"CAHd499CQb0ubfRKbaKC6Ypitq4e2ChXmTpGbKDyCVv=nrsJj=g@mail.gmail.com","threadId":"39185","inReplyTo":"xmqq382dc043.fsf@gitster.dls.corp.google.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2015-05-30T20:25:31Z","receivedAt":"2015-05-30T20:25:31Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Sat, May 30, 2015 at 2:54 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Robert Dailey <rcdailey.lists@gmail.com> writes:\n>\n>> On Sat, May 30, 2015 at 12:04 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Robert Dailey <rcdailey.lists@gmail.com> writes:\n>>>\n>>>> In the meantime I'd like to ask, do we even need to add an option for\n>>>> this? What if we just make `diff.submodule log` not use\n>>>> --first-parent? This seems like a backward compatible change in of\n>>>> itself.\n>>>\n>>> Why?  People have relied on submodule-log not to include all the\n>>> noise coming from individual commits on side branches and instead\n>>> appreciated seeing only the overview by merges of side branch topics\n>>> being listed---why is regressing the system to inconvenience these\n>>> existing users \"a backward compatible change\"?\n>>\n>> Backward compatible in the sense that it does not break existing\n>> functionality....\n>\n> And adding one-line-per-commit from side branches does break\n> existing functionality.  You seem to be arguing that more\n> information is always good and does not break existing\n> functionality, but summarizing by omitting irrelevant details *is* a\n> feature.  Do you honestly believe that a change to make the full\n> \"log -p\" output in submodule log be a \"backward compatible\" change??\n>\n>>     > Merge branch \"topic1\" into \"master\"\n>>     > Merge branch \"topic2\" into \"master\"\n>>     > Merge branch \"origin/develop\" into \"master\"\n>>     > Merge branch \"topic3\" into \"master\"\n>\n> It is not a real argument; it is something you can fix by naming\n> your branches more sensibly, which would make you a better developer\n> regardless of how submodule-log is shown.\n\nI just use git, I don't have the deep technical understanding of its\nimplementation as you may have. From my perspective I can't think of\nhow this breaks backward compatibility, or perhaps your definition of\nbackward compatibility does not align with mine.\n\nAnd please don't over generalize and misconvey what I said. I am not\nsaying more information is always good. Did you not read anything I\nwrote?\n\nAlso good branch names may help but that doesn't go into detail and\nexplain features that may have been sitting on a topic branch for\nweeks. That's not a practical solution to the problem. Also branch\nnames do not determine or influence the skill and quality of a\ndeveloper, as you seem to imply.\n\nI'll do us both a favor and end the discussion here, as I do not feel\nyou are being very patient or welcoming in the discussion. I sense\nfrustration on your side.\n\n>>>> And it's simpler to implement. I can't think of a good\n>>>> justification to add more settings to an already hugely complex\n>>>> configuration scheme for such a minor difference in behavior.\n>>>\n>>> Careful, as that argument can cut both ways.  If it is so a minor\n>>> difference in behaviour, perhaps we can do without not just an\n>>> option but a feature to omit --first-parent here.  That would be\n>>> even simpler to implement, as you do not have to do anything.\n>>\n>> I don't really understand your contrasted example here. Can you explain:\n>>\n>>     \"...we can do without not just an option but a feature to omit\n>> --first-parent...\"\n>\n> I am not sure how to phrase that any easier. Sorry.\n\nYou mean you don't want to? That's fine, if you don't have the will or\npatience to explain then I won't bother caring.\n\n> Assuming that you consider output with and without --first-parent\n> does not make much of a difference (i.e. \"minor difference in\n> behaviour\"), instead of dropping --first-parent unconditionally,\n> like you seem to be pushing with the argument, we can\n> unconditionally keep the --first-parent and do not change anything.\n> The end result would not make much of a difference either way, and\n> not doing anything is much simpler than dropping --first-parent.\n>\n> Hopefully you can see how absurd that line of reasoning is.  So do\n> not make the same argument to push for changing the behaviour\n> unconditionally.\n>\n> If you think the new behaviour can help _some_ users, then you would\n> need the feature and a knob to enable it.\n\nFirst of all, you keep calling this an argument. Perhaps it is for\nyou, since you're being absurdly rude with me and impatient with the\ndiscussion. This is a brainstorming session. My suggestions may not\nseem rational or make sense, but this is natural since I am ignorant\nof the finer details of the application.\n\nYou're really just overanalyzing my statements from a nonsensical\nperspective. I am talking about not adding more settings to an already\ncomplex set of settings. I am not justifying my feature. I think the\nfeature is just as justified as everything else. Git is FULL of tons\nof little options to cater to niche workflows.\n\nI am not fighting against having another option. In fact, that was my\nidea to begin with. I am investigating and trying to discuss all\npossible approaches and perspectives.\n\nYour attitude is not very welcoming to those that wish to contribute\nto the project. In fact, because of your attitude towards me, I will\nkindly see myself out. I do not have time to spend my free time\ndealing with this nonsense and irrational rudeness on a mailing list.\n\nThanks to Heiko and others that were more welcoming and kind.\n"},{"id":"262708","messageId":"20150602120214.GA7498@book.hvoigt.net","threadId":"39185","inReplyTo":"CAHd499CQb0ubfRKbaKC6Ypitq4e2ChXmTpGbKDyCVv=nrsJj=g@mail.gmail.com","subject":"Re: Diffing submodule does not yield complete logs for merge commits","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2015-06-02T12:02:15Z","receivedAt":"2015-06-02T12:02:15Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Sat, May 30, 2015 at 03:25:31PM -0500, Robert Dailey wrote:\n> On Sat, May 30, 2015 at 2:54 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> > Robert Dailey <rcdailey.lists@gmail.com> writes:\n> >\n> >> On Sat, May 30, 2015 at 12:04 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> >>> Robert Dailey <rcdailey.lists@gmail.com> writes:\n> >>>\n> >>>> In the meantime I'd like to ask, do we even need to add an option for\n> >>>> this? What if we just make `diff.submodule log` not use\n> >>>> --first-parent? This seems like a backward compatible change in of\n> >>>> itself.\n> >>>\n> >>> Why?  People have relied on submodule-log not to include all the\n> >>> noise coming from individual commits on side branches and instead\n> >>> appreciated seeing only the overview by merges of side branch topics\n> >>> being listed---why is regressing the system to inconvenience these\n> >>> existing users \"a backward compatible change\"?\n> >>\n> >> Backward compatible in the sense that it does not break existing\n> >> functionality....\n> >\n> > And adding one-line-per-commit from side branches does break\n> > existing functionality.  You seem to be arguing that more\n> > information is always good and does not break existing\n> > functionality, but summarizing by omitting irrelevant details *is* a\n> > feature.  Do you honestly believe that a change to make the full\n> > \"log -p\" output in submodule log be a \"backward compatible\" change??\n> >\n> >>     > Merge branch \"topic1\" into \"master\"\n> >>     > Merge branch \"topic2\" into \"master\"\n> >>     > Merge branch \"origin/develop\" into \"master\"\n> >>     > Merge branch \"topic3\" into \"master\"\n> >\n> > It is not a real argument; it is something you can fix by naming\n> > your branches more sensibly, which would make you a better developer\n> > regardless of how submodule-log is shown.\n> \n> I just use git, I don't have the deep technical understanding of its\n> implementation as you may have. From my perspective I can't think of\n> how this breaks backward compatibility, or perhaps your definition of\n> backward compatibility does not align with mine.\n\nYes from your perspective and thats the only thing what Junio is\ncriticising: Others might have a different perspective and thats why\nthis change is not backwards compatible in a general sense. For you it\nmight be but git is a very general tool.\n\n> And please don't over generalize and misconvey what I said. I am not\n> saying more information is always good. Did you not read anything I\n> wrote?\n\nI do not see where Junio did that. The only thing he was trying to do is\ngive an example why changing the default is not backwards compatible.\n\n> Also good branch names may help but that doesn't go into detail and\n> explain features that may have been sitting on a topic branch for\n> weeks. That's not a practical solution to the problem. Also branch\n> names do not determine or influence the skill and quality of a\n> developer, as you seem to imply.\n\nMany people use the feature branch name as a kind of headline for the\ntopic. So in that sense it explains or at least gives a hint what that\nfeature was about. E.g. if you have the roadmap of a library you are\nusing in mind and want to know whether the update you are about to\ncommit contains a certain feature you were waiting for, that headline\ncan be enough.\n\nYou are right: Good commit messages and general naming does\nfunctionality wise not determine your development skills in the short\nterm. On the other hand: In my experience good naming skills usually\nalign with good development skills. Long term it also means that your\ncode (and history) is easier to understand once others want to read your\ncode and build on it.\n\n> I'll do us both a favor and end the discussion here, as I do not feel\n> you are being very patient or welcoming in the discussion. I sense\n> frustration on your side.\n\nThis is a typical situation which can happen in textual communication.\nYou read something which is not there. A thing that happened to me as\nwell when I submitted my first patch on a mailing list. But when I read\nthe conversation years later I realized it was not really that offending\nas I experienced it. A general rule: People automatically appreciate\nwhat you do when they criticise to you. Criticism means that you have\ndone something good (and worth criticising) but they want to make it\nbetter. I think viewing it that way helps to not take arguments\npersonally.\n\n> >>>> And it's simpler to implement. I can't think of a good\n> >>>> justification to add more settings to an already hugely complex\n> >>>> configuration scheme for such a minor difference in behavior.\n> >>>\n> >>> Careful, as that argument can cut both ways.  If it is so a minor\n> >>> difference in behaviour, perhaps we can do without not just an\n> >>> option but a feature to omit --first-parent here.  That would be\n> >>> even simpler to implement, as you do not have to do anything.\n> >>\n> >> I don't really understand your contrasted example here. Can you explain:\n> >>\n> >>     \"...we can do without not just an option but a feature to omit\n> >> --first-parent...\"\n> >\n> > I am not sure how to phrase that any easier. Sorry.\n> \n> You mean you don't want to? That's fine, if you don't have the will or\n> patience to explain then I won't bother caring.\n\nAnd the rephrase follows...\n\n> > Assuming that you consider output with and without --first-parent\n> > does not make much of a difference (i.e. \"minor difference in\n> > behaviour\"), instead of dropping --first-parent unconditionally,\n> > like you seem to be pushing with the argument, we can\n> > unconditionally keep the --first-parent and do not change anything.\n> > The end result would not make much of a difference either way, and\n> > not doing anything is much simpler than dropping --first-parent.\n> >\n> > Hopefully you can see how absurd that line of reasoning is.  So do\n> > not make the same argument to push for changing the behaviour\n> > unconditionally.\n> >\n> > If you think the new behaviour can help _some_ users, then you would\n> > need the feature and a knob to enable it.\n> \n> First of all, you keep calling this an argument. Perhaps it is for\n> you, since you're being absurdly rude with me and impatient with the\n> discussion. This is a brainstorming session. My suggestions may not\n> seem rational or make sense, but this is natural since I am ignorant\n> of the finer details of the application.\n> \n> You're really just overanalyzing my statements from a nonsensical\n> perspective. I am talking about not adding more settings to an already\n> complex set of settings. I am not justifying my feature. I think the\n> feature is just as justified as everything else. Git is FULL of tons\n> of little options to cater to niche workflows.\n> \n> I am not fighting against having another option. In fact, that was my\n> idea to begin with. I am investigating and trying to discuss all\n> possible approaches and perspectives.\n\nJunios argument is quite absurd, but that is just to prove why we need\nanother option, nothing else. See above...\n\n> Your attitude is not very welcoming to those that wish to contribute\n> to the project. In fact, because of your attitude towards me, I will\n> kindly see myself out. I do not have time to spend my free time\n> dealing with this nonsense and irrational rudeness on a mailing list.\n> \n> Thanks to Heiko and others that were more welcoming and kind.\n\nI hope you reread this conversation more calmly and come to the same\nrealisation as me with my first patch. But opposed to me, maybe continue\nto stay with the project.\n\nCheers Heiko\n"}]}