{"thread":{"id":"43917","subject":"[PATCH] transport: report missing submodule pushes consistently on stderr","startedAt":"2016-08-23T21:46:13Z","lastAt":"2016-08-26T22:23:24Z","messageCount":4,"participants":["Stefan Beller","Leandro Lucarella"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"299993","messageId":"20160823214008.32331-1-sbeller@google.com","threadId":"43917","inReplyTo":null,"subject":"[PATCH] transport: report missing submodule pushes consistently on stderr","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-23T21:40:08Z","receivedAt":"2016-08-23T21:46:13Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The surrounding advice is printed to stderr, but the list of submodules\nis not. Make the report consistent by reporting everything to stderr.\n\nSigned-off-by: Stefan Beller \n---\n\n This fixes one of the bugs mentioned in\n https://public-inbox.org/git/CAGZ79kbkyupBJfvyX3Hj_R5ZW36+3ufOnnLC-Dpic40nPJAxDA@mail.gmail.com/T/#t\n \n How to fix the other was not as obvious to me as I do not understand the\n philosophy on verbosity in the transport code.\n \n Thanks,\n Stefan\n\n transport.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/transport.c b/transport.c\nindex cf8de6e..94d6dc3 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -771,7 +771,7 @@ static void die_with_unpushed_submodules(struct string_list *needs_pushing)\n \tfprintf(stderr, _(\"The following submodule paths contain changes that can\\n\"\n \t\t\t\"not be found on any remote:\\n\"));\n \tfor (i = 0; i < needs_pushing->nr; i++)\n-\t\tprintf(\" %s\\n\", needs_pushing->items[i].string);\n+\t\tfprintf(stderr, \" %s\\n\", needs_pushing->items[i].string);\n \tfprintf(stderr, _(\"\\nPlease try\\n\\n\"\n \t\t\t \"\tgit push --recurse-submodules=on-demand\\n\\n\"\n \t\t\t \"or cd to the path and use\\n\\n\"\n-- \n2.10.0.rc1.1.g1ceb01a\n\n"},{"id":"300039","messageId":"20160824122828.0c2688e8@labs-064.localdomain","threadId":"43917","inReplyTo":"20160823214008.32331-1-sbeller@google.com","subject":"Re: [PATCH] transport: report missing submodule pushes consistently on stderr","fromName":"Leandro Lucarella","fromEmail":"leandro.lucarella@sociomantic.com","sentAt":"2016-08-24T10:28:28Z","receivedAt":"2016-08-24T11:00:22Z","isPatch":true,"sender":{"key":"leandro.lucarella@sociomantic.com","avatar":"https://gravatar.com/avatar/b8884950a0d028195af6bb849af0136915e795824f1d888a9599af7ce3d174af?d=mp&s=160"},"body":"On Tue, 23 Aug 2016 14:40:08 -0700\nStefan Beller wrote:\n> The surrounding advice is printed to stderr, but the list of\n> submodules is not. Make the report consistent by reporting everything\n> to stderr.\n> \n> Signed-off-by: Stefan Beller \n> ---\n> \n> This fixes one of the bugs mentioned in\n> https://public-inbox.org/git/CAGZ79kbkyupBJfvyX3Hj_R5ZW36+3ufOnnLC-Dpic40nPJAxDA@mail.gmail.com/T/#t\n> \n> How to fix the other was not as obvious to me as I do not\n> understand the philosophy on verbosity in the transport code.\n\nI had a look and I would say just enclose all the fprintf() inside a:\n\n\tif (transport->verbose > 0)\n\nBut then this is the first time I look at the code. I was about to send\na patch too but it will conflict with this one :)\n\nAnyway, thanks for the quick fix to the inconsistent printing with\n--quiet.\n\n\n-- \nLeandro Lucarella\nTechnical Development Lead\nSociomantic Labs GmbH \n"},{"id":"300076","messageId":"CAGZ79kb1ur=s+FYcdCh+obKyfaozX=adAPESu9G9FQxVtLTDbg@mail.gmail.com","threadId":"43917","inReplyTo":"20160824122828.0c2688e8@labs-064.localdomain","subject":"Re: [PATCH] transport: report missing submodule pushes consistently on stderr","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-24T16:35:10Z","receivedAt":"2016-08-24T16:35:15Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 24, 2016 at 3:28 AM, Leandro Lucarella\n wrote:\n> On Tue, 23 Aug 2016 14:40:08 -0700\n> Stefan Beller wrote:\n>> The surrounding advice is printed to stderr, but the list of\n>> submodules is not. Make the report consistent by reporting everything\n>> to stderr.\n>>\n>> Signed-off-by: Stefan Beller \n>> ---\n>>\n>> This fixes one of the bugs mentioned in\n>> https://public-inbox.org/git/CAGZ79kbkyupBJfvyX3Hj_R5ZW36+3ufOnnLC-Dpic40nPJAxDA@mail.gmail.com/T/#t\n>>\n>> How to fix the other was not as obvious to me as I do not\n>> understand the philosophy on verbosity in the transport code.\n>\n> I had a look and I would say just enclose all the fprintf() inside a:\n>\n> if (transport->verbose > 0)\n>\n> But then this is the first time I look at the code. I was about to send\n> a patch too but it will conflict with this one :)\n\nWell you can still send a patch :)\n\nWe have\n\n int verbose = (transport->verbose > 0);\n int quiet = (transport->verbose < 0);\n\nSo you're suggesting to only print these warnings when the\nuser asked for explicit verbose?\n\nA few lines before the call to die_with_unpushed_submodules we have\n\n die (\"Failed to push all needed submodules!\");\n\nwhich would also need a wrapping like\n\n if (quiet)\n return -1;\n else\n die(...);\n\nThanks,\nStefan\n"},{"id":"300309","messageId":"CAGZ79kaT79WBfwbRw5bw9LOQgHThVOpn66siwr14kU7=ju2WZA@mail.gmail.com","threadId":"43917","inReplyTo":"CAGZ79kb1ur=s+FYcdCh+obKyfaozX=adAPESu9G9FQxVtLTDbg@mail.gmail.com","subject":"Re: [PATCH] transport: report missing submodule pushes consistently on stderr","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-26T22:21:53Z","receivedAt":"2016-08-26T22:23:24Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 24, 2016 at 9:35 AM, Stefan Beller wrote:\n> On Wed, Aug 24, 2016 at 3:28 AM, Leandro Lucarella\n> wrote:\n>> On Tue, 23 Aug 2016 14:40:08 -0700\n>> Stefan Beller wrote:\n>>> The surrounding advice is printed to stderr, but the list of\n>>> submodules is not. Make the report consistent by reporting everything\n>>> to stderr.\n>>>\n>>> Signed-off-by: Stefan Beller \n>>> ---\n\nSo we seem to have dropped the ball on the followup; this patch nevertheless\nis a good idea?\n"}]}