{"thread":{"id":"45542","subject":"[RFC PATCH] change default for status.submoduleSummary to true","startedAt":"2017-03-30T01:20:41Z","lastAt":"2017-03-30T18:02:58Z","messageCount":4,"participants":["Stefan Beller","Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"315727","messageId":"20170330012025.22122-1-sbeller@google.com","threadId":"45542","inReplyTo":null,"subject":"[RFC PATCH] change default for status.submoduleSummary to true","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-03-30T01:20:25Z","receivedAt":"2017-03-30T01:20:41Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"A user complained about the workflow with submodules:\n> Re submodules pain, I've seen a lot of people get confused about\n> how and when to commit submodule changes. The main thing missing\n> in the related UIs is some way to summarize the subproject commit\n> diff in a human readable way. Maybe last log message would be better\n> than just sha?\n\nWe could advise all the confused users to turn on\nstatus.submoduleSummary.  However there is no downside from turning\nit on by default apart from a slight change in behavior and bit\nlonger output of git-status and the help in git-commit.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\n Maybe we can merge this early after 2.13, so we have a longer time frame\n in which people may react to this change of a default?\n \n Thanks,\n Stefan\n\n Documentation/config.txt | 2 +-\n builtin/commit.c         | 2 ++\n 2 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1df1965457..34d4735414 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2984,7 +2984,7 @@ This variable can be overridden with the -u|--untracked-files option\n of linkgit:git-status[1] and linkgit:git-commit[1].\n \n status.submoduleSummary::\n-\tDefaults to false.\n+\tDefaults to true.\n \tIf this is set to a non zero number or true (identical to -1 or an\n \tunlimited number), the submodule summary will be enabled and a\n \tsummary of commits for modified submodules will be shown (see\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 4e288bc513..833a651013 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1388,6 +1388,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \n \ts.ignore_submodule_arg = ignore_submodule_arg;\n \ts.status_format = status_format;\n+\ts.submodule_summary = -1;\n \ts.verbose = verbose;\n \n \twt_status_collect(&s);\n@@ -1650,6 +1651,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tstatus_init_config(&s, git_commit_config);\n \tstatus_format = STATUS_FORMAT_NONE; /* Ignore status.short */\n \ts.colopts = 0;\n+\ts.submodule_summary = -1;\n \n \tif (get_sha1(\"HEAD\", oid.hash))\n \t\tcurrent_head = NULL;\n-- \n2.12.2.511.g2abb8caf66\n\n"},{"id":"315728","messageId":"CA+P7+xodO7=0TQPZPseDPw_Owsdy2jrCUKpP0G_+j-bz51KNVA@mail.gmail.com","threadId":"45542","inReplyTo":"20170330012025.22122-1-sbeller@google.com","subject":"Re: [RFC PATCH] change default for status.submoduleSummary to true","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-03-30T01:33:13Z","receivedAt":"2017-03-30T01:33:41Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, Mar 29, 2017 at 6:20 PM, Stefan Beller <sbeller@google.com> wrote:\n> A user complained about the workflow with submodules:\n>> Re submodules pain, I've seen a lot of people get confused about\n>> how and when to commit submodule changes. The main thing missing\n>> in the related UIs is some way to summarize the subproject commit\n>> diff in a human readable way. Maybe last log message would be better\n>> than just sha?\n>\n> We could advise all the confused users to turn on\n> status.submoduleSummary.  However there is no downside from turning\n> it on by default apart from a slight change in behavior and bit\n> longer output of git-status and the help in git-commit.\n>\n\nMakes sense to me.\n\nThanks,\nJake\n\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>\n>  Maybe we can merge this early after 2.13, so we have a longer time frame\n>  in which people may react to this change of a default?\n>\n>  Thanks,\n>  Stefan\n>\n>  Documentation/config.txt | 2 +-\n>  builtin/commit.c         | 2 ++\n>  2 files changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 1df1965457..34d4735414 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -2984,7 +2984,7 @@ This variable can be overridden with the -u|--untracked-files option\n>  of linkgit:git-status[1] and linkgit:git-commit[1].\n>\n>  status.submoduleSummary::\n> -       Defaults to false.\n> +       Defaults to true.\n>         If this is set to a non zero number or true (identical to -1 or an\n>         unlimited number), the submodule summary will be enabled and a\n>         summary of commits for modified submodules will be shown (see\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 4e288bc513..833a651013 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1388,6 +1388,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n>\n>         s.ignore_submodule_arg = ignore_submodule_arg;\n>         s.status_format = status_format;\n> +       s.submodule_summary = -1;\n>         s.verbose = verbose;\n>\n>         wt_status_collect(&s);\n> @@ -1650,6 +1651,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>         status_init_config(&s, git_commit_config);\n>         status_format = STATUS_FORMAT_NONE; /* Ignore status.short */\n>         s.colopts = 0;\n> +       s.submodule_summary = -1;\n>\n>         if (get_sha1(\"HEAD\", oid.hash))\n>                 current_head = NULL;\n> --\n> 2.12.2.511.g2abb8caf66\n>\n"},{"id":"315746","messageId":"xmqqmvc37120.fsf@gitster.mtv.corp.google.com","threadId":"45542","inReplyTo":"20170330012025.22122-1-sbeller@google.com","subject":"Re: [RFC PATCH] change default for status.submoduleSummary to true","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-30T06:04:23Z","receivedAt":"2017-03-30T06:04:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> A user complained about the workflow with submodules:\n>> Re submodules pain, I've seen a lot of people get confused about\n>> how and when to commit submodule changes. The main thing missing\n>> in the related UIs is some way to summarize the subproject commit\n>> diff in a human readable way. Maybe last log message would be better\n>> than just sha?\n>\n> We could advise all the confused users to turn on\n> status.submoduleSummary.  However there is no downside from turning\n> it on by default apart from a slight change in behavior and bit\n> longer output of git-status and the help in git-commit.\n\nIs \"there is no downside\" substantiated or just hand-waving?\nAny pros-and-cons analysis, e.g. performance implications, etc.?\n\n"},{"id":"315800","messageId":"CAGZ79kbnq0uh9VGwru0JjW8_gJvUtvnt2fsCHNAyBiM=u6ENmw@mail.gmail.com","threadId":"45542","inReplyTo":"xmqqmvc37120.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC PATCH] change default for status.submoduleSummary to true","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-03-30T18:02:53Z","receivedAt":"2017-03-30T18:02:58Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Mar 29, 2017 at 11:04 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> A user complained about the workflow with submodules:\n>>> Re submodules pain, I've seen a lot of people get confused about\n>>> how and when to commit submodule changes. The main thing missing\n>>> in the related UIs is some way to summarize the subproject commit\n>>> diff in a human readable way. Maybe last log message would be better\n>>> than just sha?\n>>\n>> We could advise all the confused users to turn on\n>> status.submoduleSummary.  However there is no downside from turning\n>> it on by default apart from a slight change in behavior and bit\n>> longer output of git-status and the help in git-commit.\n>\n> Is \"there is no downside\" substantiated or just hand-waving?\n\nIt's the best kind of hand waving.\n\n> Any pros-and-cons analysis, e.g. performance implications, etc.?\n\nPerformance will be terrible. The submodule summary is gathered by\nrunning \"git submodule summary\", which is shell (that is slower than\nthe C code in status).\n\nIn gerrit (which has 6 submodules, that I modified for the test by adding\none commit on top of each), we find\n\n$ time git -c status.submoduleSummary=false status\n...\nreal 0m0.043s\n\n$ time git -c status.submoduleSummary=true status\n...\nreal 0m0.359s\n\nSo it is slower by an order of magnitude.\nMaybe it is not the right time to propose this patch then, but we'\nrather want to wait until \"git submodule summary\" is converted to C.\n(Maybe we can even parallelize the data gathering before output.)\n\nEven for git.git (which has no submodules), we have\n\n$ time git -c status.submoduleSummary=false status\n...\nreal 0m0.014s\n$ time git -c status.submoduleSummary=true status\n...\nreal 0m0.125s\n\nSo additionally to porting that part to C, we may want to condition it\nto a new function \"int do_we_have_submodules_at_all(void)\",\nwhich has a similar heuristic as the once proposed patch\nto recurse into submodules all the time\nhttps://public-inbox.org/git/20161006193725.31553-3-sbeller@google.com/\n\nNow that I explained why this patch is unacceptable as-is,\nlet's think of potential upsides:\n\ngit-status is the one command to deliver a summary on the state\nof a repo with the granularity on the commit\n(e.g. it reports the relation to its upstream tracking branch)\nand file level (which files are change, untracked).\n\nAnd I think a submodule is a weird between-state of these\ntwo as it is seen like a file in the superproject, but it also\nhas commits, which are the smallest unit of logical change.\n\nSo maybe we'd want to report just a number in git-status,\nwhich indicates how much the current detached head\nis behind/before the remote tracking branch, and if it can be\nfast forwarded.\n\nThanks,\nStefan\n"}]}