{"thread":{"id":"47374","subject":"[PATCH v1] progress: print progress output for all operations taking longer than 2s","startedAt":"2017-12-04T20:37:31Z","lastAt":"2017-12-05T12:28:29Z","messageCount":11,"participants":["lars.schneider@autodesk.com","Jeff King","Junio C Hamano","Lars Schneider","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"334086","messageId":"20171204203647.30546-1-lars.schneider@autodesk.com","threadId":"47374","inReplyTo":null,"subject":"[PATCH v1] progress: print progress output for all operations taking longer than 2s","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2017-12-04T20:36:47Z","receivedAt":"2017-12-04T20:37:31Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\nIn 180a9f2 we implemented a progress API which suppresses the progress\noutput if the progress has reached a specified percentage threshold\nwithin a given time frame. In 8aade10 we simplified the API and set the\nthreshold to 0% and the time frame to 2 seconds for all delayed progress\noperations. That means we would only see a progress output if we still\nhave 0% progress after 2 seconds. Consequently, only operations that\nhave a very slow start would show the progress output at all.\n\nRemove the threshold entirely and print the progress output for all\noperations that take longer than 2 seconds.\n\nSigned-off-by: Lars Schneider <larsxschneider@gmail.com>\n---\n\nHi,\n\na few weeks ago I was puzzled why the progress output is not shown in\ncertain situations [1]. I debugged the issue a bit today and came up\nwith this patch as solution. It is entirely possible that I misunderstood\nthe intentions of the progress API and therefore my patch is bogus.\nIn this case, please treat this email as RFC.\n\nThanks,\nLars\n\n\n[1] https://public-inbox.org/git/DC84FB2E-A26E-4957-B5FA-BE6DDEC3411B@gmail.com/\n\n\nNotes:\n    Base Commit: 1a4e40aa5d (1a4e40aa5dc16564af879142ba9dfbbb88d1e5ff)\n    Diff on Web: https://github.com/larsxschneider/git/commit/3e5fdc512a\n    Checkout:    git fetch https://github.com/larsxschneider/git progress-fix-v1 && git checkout 3e5fdc512a\n\n progress.c | 18 +++---------------\n 1 file changed, 3 insertions(+), 15 deletions(-)\n\ndiff --git a/progress.c b/progress.c\nindex 289678d43d..7fa1b0f235 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -34,7 +34,6 @@ struct progress {\n \tunsigned total;\n \tunsigned last_percent;\n \tunsigned delay;\n-\tunsigned delayed_percent_threshold;\n \tstruct throughput *throughput;\n \tuint64_t start_ns;\n };\n@@ -86,16 +85,6 @@ static int display(struct progress *progress, unsigned n, const char *done)\n \tif (progress->delay) {\n \t\tif (!progress_update || --progress->delay)\n \t\t\treturn 0;\n-\t\tif (progress->total) {\n-\t\t\tunsigned percent = n * 100 / progress->total;\n-\t\t\tif (percent > progress->delayed_percent_threshold) {\n-\t\t\t\t/* inhibit this progress report entirely */\n-\t\t\t\tclear_progress_signal();\n-\t\t\t\tprogress->delay = -1;\n-\t\t\t\tprogress->total = 0;\n-\t\t\t\treturn 0;\n-\t\t\t}\n-\t\t}\n \t}\n\n \tprogress->last_value = n;\n@@ -206,7 +195,7 @@ int display_progress(struct progress *progress, unsigned n)\n }\n\n static struct progress *start_progress_delay(const char *title, unsigned total,\n-\t\t\t\t\t     unsigned percent_threshold, unsigned delay)\n+\t\t\t\t\t     unsigned delay)\n {\n \tstruct progress *progress = malloc(sizeof(*progress));\n \tif (!progress) {\n@@ -219,7 +208,6 @@ static struct progress *start_progress_delay(const char *title, unsigned total,\n \tprogress->total = total;\n \tprogress->last_value = -1;\n \tprogress->last_percent = -1;\n-\tprogress->delayed_percent_threshold = percent_threshold;\n \tprogress->delay = delay;\n \tprogress->throughput = NULL;\n \tprogress->start_ns = getnanotime();\n@@ -229,12 +217,12 @@ static struct progress *start_progress_delay(const char *title, unsigned total,\n\n struct progress *start_delayed_progress(const char *title, unsigned total)\n {\n-\treturn start_progress_delay(title, total, 0, 2);\n+\treturn start_progress_delay(title, total, 2);\n }\n\n struct progress *start_progress(const char *title, unsigned total)\n {\n-\treturn start_progress_delay(title, total, 0, 0);\n+\treturn start_progress_delay(title, total, 0);\n }\n\n void stop_progress(struct progress **p_progress)\n--\n2.15.1\n\n"},{"id":"334089","messageId":"20171204213350.GA21552@sigill.intra.peff.net","threadId":"47374","inReplyTo":"20171204203647.30546-1-lars.schneider@autodesk.com","subject":"Re: [PATCH v1] progress: print progress output for all operations taking longer than 2s","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-12-04T21:33:50Z","receivedAt":"2017-12-04T21:33:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 04, 2017 at 09:36:47PM +0100, lars.schneider@autodesk.com wrote:\n\n> From: Lars Schneider <larsxschneider@gmail.com>\n> \n> In 180a9f2 we implemented a progress API which suppresses the progress\n> output if the progress has reached a specified percentage threshold\n> within a given time frame. In 8aade10 we simplified the API and set the\n> threshold to 0% and the time frame to 2 seconds for all delayed progress\n> operations. That means we would only see a progress output if we still\n> have 0% progress after 2 seconds. Consequently, only operations that\n> have a very slow start would show the progress output at all.\n> \n> Remove the threshold entirely and print the progress output for all\n> operations that take longer than 2 seconds.\n\nGood catch.\n\nI was surprised at first to see 8aade10 as the culprit here, because it\nwas supposed to be a mechanical conversion. I.e.:\n\n  start_progress_delay(\"foo\", 0, 0, 2);\n  start_progress_delay(\"bar\", nr, 50, 1);\n\nwould both call the wrapper with a 0-percent delay threshold.\n\nAnd now call sites like the first one still works, but the second one is\nbroken.\n\nThe problem is that commit got the meaning of the threshold parameter\ntotally backwards. It is \"only show progress if we have not gotten past\nthis percent\". Which means that passing \"0\" is nonsense, as you\ndiscovered.\n\nBut wait, why did we have all those calls using \"0\" in the first place,\nand why did they work? The answer is that we can only look at the\nthreshold at all when we know the total, since otherwise we can't\ncompute a percentage. So that first call should logically have been:\n\n  start_progress_delay(\"foo\", 0, 100, 2);\n\nI.e., 100% to indicate that we always show the progress after 2 seconds\nregardless. But since our total was \"0\", we never looked at the\nthreshold at all. But they misled us into thinking that \"0\" was the way\nto say \"always show this progress after 2 seconds\".\n\nSo the minimal fix is actually:\n\ndiff --git a/progress.c b/progress.c\nindex 289678d43d..b774cb1cd1 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -229,7 +229,7 @@ static struct progress *start_progress_delay(const char *title, unsigned total,\n \n struct progress *start_delayed_progress(const char *title, unsigned total)\n {\n-\treturn start_progress_delay(title, total, 0, 2);\n+\treturn start_progress_delay(title, total, 100, 2);\n }\n \n struct progress *start_progress(const char *title, unsigned total)\n\nThat keeps \"start_progress_delay\" functioning properly, but makes\ncallers of the wrapper use a sane threshold.\n\nThat said, after 8aade10 there are literally no callers that use a\nthreshold other than what is set here. So we could just rip out the\n\"threshold\" feature entirely, which is what your patch is doing.\n\nI'm OK with that route, but I think we would want to explain that\ndecision in the commit message a bit better.\n\n> a few weeks ago I was puzzled why the progress output is not shown in\n> certain situations [1]. I debugged the issue a bit today and came up\n> with this patch as solution. It is entirely possible that I misunderstood\n> the intentions of the progress API and therefore my patch is bogus.\n> In this case, please treat this email as RFC.\n\nNo, I think 8aade10 misunderstood the progress API. ;)\n\n-Peff\n"},{"id":"334091","messageId":"xmqqzi6ykwhn.fsf@gitster.mtv.corp.google.com","threadId":"47374","inReplyTo":"20171204203647.30546-1-lars.schneider@autodesk.com","subject":"Re: [PATCH v1] progress: print progress output for all operations taking longer than 2s","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-04T21:35:00Z","receivedAt":"2017-12-04T21:35:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"lars.schneider@autodesk.com writes:\n\n> In 180a9f2 we implemented a progress API which suppresses the progress\n> output if the progress has reached a specified percentage threshold\n> within a given time frame. In 8aade10 we simplified the API and set the\n> threshold to 0% and the time frame to 2 seconds for all delayed progress\n> operations. That means we would only see a progress output if we still\n> have 0% progress after 2 seconds. Consequently, only operations that\n> have a very slow start would show the progress output at all.\n>\n> Remove the threshold entirely and print the progress output for all\n> operations that take longer than 2 seconds.\n\nIsn't this likely to result in much chattier progress output (read:\nregression) forplaces whose the (P, N) was (0%, 2s) before 8aade107\n(\"progress: simplify \"delayed\" progress API\", 2017-08-19)?\n\nBefore or after that change, the places that passed (0%, 2s)\nrefrained from showing any progress, if at least 1 per-cent of the\nwork has finished at 2-second mark.  With this change, they will\nsuddenly start showing progress after 2-second mark, even if they\ncompleted that much work already.\n\nThe places that did change the behaviour with the cited change are\nthe ones that used parameters different from (0%, 2s).  \"git blame\",\n\"diffcore-rename\" and \"unpack-trees\" seem to be among them; they\nused (50%, 1s), and we'd have seen the progress meter after 1s,\nunless half of the work is already done by that time.  By replacing\nthat with (0%, 2s), the change made it a lot less likely to trigger.\n\nThe analysis in the cited commit log claims that (50%, 1s) is\nequivalent to (0%, 2s) when the workload is smooth, but I think that\nmath is bogus X-<.\n\nAnd the one in prune-packed, which used to be (95%, 2s), is quite\ndifferent from the value after the simplification.  We deliberately\nmade it 95 times more unlikely to trigger with that commit---it used\nto be that unless 95% of work is already done, we saw progress\nstarting at 2-second mark, but now we see progress only when less\nthan 1% of work is done at 2-second mark.\n\n"},{"id":"334092","messageId":"xmqqvahmkwbi.fsf@gitster.mtv.corp.google.com","threadId":"47374","inReplyTo":"20171204213350.GA21552@sigill.intra.peff.net","subject":"Re: [PATCH v1] progress: print progress output for all operations taking longer than 2s","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-04T21:38:41Z","receivedAt":"2017-12-04T21:38:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> So the minimal fix is actually:\n>\n> diff --git a/progress.c b/progress.c\n> index 289678d43d..b774cb1cd1 100644\n> --- a/progress.c\n> +++ b/progress.c\n> @@ -229,7 +229,7 @@ static struct progress *start_progress_delay(const char *title, unsigned total,\n>  \n>  struct progress *start_delayed_progress(const char *title, unsigned total)\n>  {\n> -\treturn start_progress_delay(title, total, 0, 2);\n> +\treturn start_progress_delay(title, total, 100, 2);\n>  }\n\nThat makes a lot more sense to me (at least from a cursory\ncomparison between the two approaches).\n\n"},{"id":"334098","messageId":"20171204220228.GA29422@sigill.intra.peff.net","threadId":"47374","inReplyTo":"xmqqvahmkwbi.fsf@gitster.mtv.corp.google.com","subject":"[PATCH 0/2] fix v2.15 progress regression","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-12-04T22:02:28Z","receivedAt":"2017-12-04T22:02:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 04, 2017 at 01:38:41PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > So the minimal fix is actually:\n> >\n> > diff --git a/progress.c b/progress.c\n> > index 289678d43d..b774cb1cd1 100644\n> > --- a/progress.c\n> > +++ b/progress.c\n> > @@ -229,7 +229,7 @@ static struct progress *start_progress_delay(const char *title, unsigned total,\n> >  \n> >  struct progress *start_delayed_progress(const char *title, unsigned total)\n> >  {\n> > -\treturn start_progress_delay(title, total, 0, 2);\n> > +\treturn start_progress_delay(title, total, 100, 2);\n> >  }\n> \n> That makes a lot more sense to me (at least from a cursory\n> comparison between the two approaches).\n\nHere's what I think we should do: fix the bug in the minimal way, and\nthen drop the useless code. It's worth doing in two steps, because we\nmay decide to resurrect the feature later, and it would then just be a\nstraight revert of the second commit.\n\n  [1/2]: progress: set default delay threshold to 100%, not 0%\n  [2/2]: progress: drop delay-threshold code\n\n progress.c | 24 +++++-------------------\n 1 file changed, 5 insertions(+), 19 deletions(-)\n\nThis regression is in v2.15, so this probably ought to go to maint (at\nleast the first part, though I think the second should have no\nuser-visible effects).\n\n-Peff\n"},{"id":"334100","messageId":"20171204220523.GA18828@sigill.intra.peff.net","threadId":"47374","inReplyTo":"20171204220228.GA29422@sigill.intra.peff.net","subject":"[PATCH 1/2] progress: set default delay threshold to 100%, not 0%","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-12-04T22:05:23Z","receivedAt":"2017-12-04T22:05:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Commit 8aade107dd (progress: simplify \"delayed\" progress\nAPI, 2017-08-19) dropped the parameter by which callers\ncould say \"show my progress only if I haven't passed M%\nprogress after N seconds\". The intent was to just show\nnothing for 2 seconds, and then always progress after that.\n\nBut we flipped the logic in the wrapper: it sets M=0,\nmeaning that we'd almost _never_ show progress after 2\nseconds, since we'd generally have made some progress. This\nshould have been 100%, not 0%.\n\nWe were fooled by existing calls like:\n\n  start_progress_delay(\"foo\", 0, 0, 2);\n\nwhich behaved this way. The trick is that the first \"0\"\nthere is \"how many items total\", and there zero means \"we\ndon't know\". And without knowing that, we cannot compute a\ncompleted percent at all, and we ignored the threshold\nparameter entirely! Modeling our wrapper after that broke\ncallers which pass a non-zero value for \"total\".\n\nWe can switch to the intended behavior by using \"100\" in the\nwrapper call.\n\nReported-by: Lars Schneider <larsxschneider@gmail.com>\nSigned-off-by: Jeff King <peff@peff.net>\n---\n progress.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/progress.c b/progress.c\nindex 289678d43d..b774cb1cd1 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -229,7 +229,7 @@ static struct progress *start_progress_delay(const char *title, unsigned total,\n \n struct progress *start_delayed_progress(const char *title, unsigned total)\n {\n-\treturn start_progress_delay(title, total, 0, 2);\n+\treturn start_progress_delay(title, total, 100, 2);\n }\n \n struct progress *start_progress(const char *title, unsigned total)\n-- \n2.15.0.691.g622df76569\n\n"},{"id":"334101","messageId":"20171204220700.GB18828@sigill.intra.peff.net","threadId":"47374","inReplyTo":"20171204220228.GA29422@sigill.intra.peff.net","subject":"[PATCH 2/2] progress: drop delay-threshold code","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-12-04T22:07:00Z","receivedAt":"2017-12-04T22:07:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\nSince 180a9f2268 (provide a facility for \"delayed\" progress\nreporting, 2007-04-20), the progress code has allowed\ncallers to skip showing progress if they have reached a\npercentage-threshold of the total work before the delay\nperiod passes.\n\nBut since 8aade107dd (progress: simplify \"delayed\" progress\nAPI, 2017-08-19), that parameter is not available to outside\ncallers (we always passed zero after that commit, though\nthat was corrected in the previous commit to \"100%\").\n\nLet's drop the threshold code, which never triggers in\nany meaningful way.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI tweaked your patch slightly to clean up the now-simplified\nconditional.\n\n progress.c | 24 +++++-------------------\n 1 file changed, 5 insertions(+), 19 deletions(-)\n\ndiff --git a/progress.c b/progress.c\nindex b774cb1cd1..5f87f4568f 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -34,7 +34,6 @@ struct progress {\n \tunsigned total;\n \tunsigned last_percent;\n \tunsigned delay;\n-\tunsigned delayed_percent_threshold;\n \tstruct throughput *throughput;\n \tuint64_t start_ns;\n };\n@@ -83,20 +82,8 @@ static int display(struct progress *progress, unsigned n, const char *done)\n {\n \tconst char *eol, *tp;\n \n-\tif (progress->delay) {\n-\t\tif (!progress_update || --progress->delay)\n-\t\t\treturn 0;\n-\t\tif (progress->total) {\n-\t\t\tunsigned percent = n * 100 / progress->total;\n-\t\t\tif (percent > progress->delayed_percent_threshold) {\n-\t\t\t\t/* inhibit this progress report entirely */\n-\t\t\t\tclear_progress_signal();\n-\t\t\t\tprogress->delay = -1;\n-\t\t\t\tprogress->total = 0;\n-\t\t\t\treturn 0;\n-\t\t\t}\n-\t\t}\n-\t}\n+\tif (progress->delay && (!progress_update || --progress->delay))\n+\t\treturn 0;\n \n \tprogress->last_value = n;\n \ttp = (progress->throughput) ? progress->throughput->display.buf : \"\";\n@@ -206,7 +193,7 @@ int display_progress(struct progress *progress, unsigned n)\n }\n \n static struct progress *start_progress_delay(const char *title, unsigned total,\n-\t\t\t\t\t     unsigned percent_threshold, unsigned delay)\n+\t\t\t\t\t     unsigned delay)\n {\n \tstruct progress *progress = malloc(sizeof(*progress));\n \tif (!progress) {\n@@ -219,7 +206,6 @@ static struct progress *start_progress_delay(const char *title, unsigned total,\n \tprogress->total = total;\n \tprogress->last_value = -1;\n \tprogress->last_percent = -1;\n-\tprogress->delayed_percent_threshold = percent_threshold;\n \tprogress->delay = delay;\n \tprogress->throughput = NULL;\n \tprogress->start_ns = getnanotime();\n@@ -229,12 +215,12 @@ static struct progress *start_progress_delay(const char *title, unsigned total,\n \n struct progress *start_delayed_progress(const char *title, unsigned total)\n {\n-\treturn start_progress_delay(title, total, 100, 2);\n+\treturn start_progress_delay(title, total, 2);\n }\n \n struct progress *start_progress(const char *title, unsigned total)\n {\n-\treturn start_progress_delay(title, total, 0, 0);\n+\treturn start_progress_delay(title, total, 0);\n }\n \n void stop_progress(struct progress **p_progress)\n-- \n2.15.0.691.g622df76569\n"},{"id":"334103","messageId":"xmqqindmkua3.fsf@gitster.mtv.corp.google.com","threadId":"47374","inReplyTo":"20171204220228.GA29422@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] fix v2.15 progress regression","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-04T22:22:44Z","receivedAt":"2017-12-04T22:22:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Here's what I think we should do: fix the bug in the minimal way, and\n> then drop the useless code. It's worth doing in two steps, because we\n> may decide to resurrect the feature later, and it would then just be a\n> straight revert of the second commit.\n\nYup.  Thanks; will queue.\n"},{"id":"334142","messageId":"BBE0F8C1-2B9E-42B6-AE47-90C8A62A4F84@gmail.com","threadId":"47374","inReplyTo":"20171204220700.GB18828@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] progress: drop delay-threshold code","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-12-05T10:37:30Z","receivedAt":"2017-12-05T10:37:36Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 04 Dec 2017, at 23:07, Jeff King <peff@peff.net> wrote:\n> \n> From: Lars Schneider <larsxschneider@gmail.com>\n> \n> Since 180a9f2268 (provide a facility for \"delayed\" progress\n> reporting, 2007-04-20), the progress code has allowed\n> callers to skip showing progress if they have reached a\n> percentage-threshold of the total work before the delay\n> period passes.\n> \n> But since 8aade107dd (progress: simplify \"delayed\" progress\n> API, 2017-08-19), that parameter is not available to outside\n> callers (we always passed zero after that commit, though\n> that was corrected in the previous commit to \"100%\").\n> \n> Let's drop the threshold code, which never triggers in\n> any meaningful way.\n> \n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> I tweaked your patch slightly to clean up the now-simplified\n> conditional.\n\nYour first patch (\"progress: set default delay threshold to 100%, not 0%\")\nas well as the modifications to this one look good to me. Feel free\nto add my \"Signed-off-by: Lars Schneider <larsxschneider@gmail.com>\".\n\nThanks,\nLars\n\n\nPS: How do you generate the commit references \"hash (first line, date)\"?\nGit log pretty print?\n"},{"id":"334143","messageId":"CACBZZX63ZhOHXmgerpJHc+ri_-w=QUyOQ7feWHxyDwPhN8hnDg@mail.gmail.com","threadId":"47374","inReplyTo":"BBE0F8C1-2B9E-42B6-AE47-90C8A62A4F84@gmail.com","subject":"Re: [PATCH 2/2] progress: drop delay-threshold code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-12-05T11:10:05Z","receivedAt":"2017-12-05T11:10:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Dec 5, 2017 at 11:37 AM, Lars Schneider\n<larsxschneider@gmail.com> wrote:\n>\n>> On 04 Dec 2017, at 23:07, Jeff King <peff@peff.net> wrote:\n>>\n>> From: Lars Schneider <larsxschneider@gmail.com>\n>>\n>> Since 180a9f2268 (provide a facility for \"delayed\" progress\n>> reporting, 2007-04-20), the progress code has allowed\n>> callers to skip showing progress if they have reached a\n>> percentage-threshold of the total work before the delay\n>> period passes.\n>>\n>> But since 8aade107dd (progress: simplify \"delayed\" progress\n>> API, 2017-08-19), that parameter is not available to outside\n>> callers (we always passed zero after that commit, though\n>> that was corrected in the previous commit to \"100%\").\n>>\n>> Let's drop the threshold code, which never triggers in\n>> any meaningful way.\n>>\n>> Signed-off-by: Jeff King <peff@peff.net>\n>> ---\n>> I tweaked your patch slightly to clean up the now-simplified\n>> conditional.\n>\n> Your first patch (\"progress: set default delay threshold to 100%, not 0%\")\n> as well as the modifications to this one look good to me. Feel free\n> to add my \"Signed-off-by: Lars Schneider <larsxschneider@gmail.com>\".\n>\n> Thanks,\n> Lars\n>\n>\n> PS: How do you generate the commit references \"hash (first line, date)\"?\n> Git log pretty print?\n\n$ git grep -A5 'Copy commit summary' Documentation/SubmittingPatches\nDocumentation/SubmittingPatches:151:The \"Copy commit summary\" command\nof gitk can be used to obtain this\nDocumentation/SubmittingPatches-152-format, or this invocation of `git show`:\nDocumentation/SubmittingPatches-153-\nDocumentation/SubmittingPatches-154-....\nDocumentation/SubmittingPatches-155-    git show -s --date=short\n--pretty='format:%h (\"%s\", %ad)' <commit>\n"},{"id":"334147","messageId":"05818F78-7A4C-4ABE-B1C5-87D6991C98D8@gmail.com","threadId":"47374","inReplyTo":"CACBZZX63ZhOHXmgerpJHc+ri_-w=QUyOQ7feWHxyDwPhN8hnDg@mail.gmail.com","subject":"Re: [PATCH 2/2] progress: drop delay-threshold code","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-12-05T12:28:21Z","receivedAt":"2017-12-05T12:28:29Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 05 Dec 2017, at 12:10, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> \n> On Tue, Dec 5, 2017 at 11:37 AM, Lars Schneider\n> <larsxschneider@gmail.com> wrote:\n>> \n>>> On 04 Dec 2017, at 23:07, Jeff King <peff@peff.net> wrote:\n>>> \n>>> From: Lars Schneider <larsxschneider@gmail.com>\n>>> \n>>> Since 180a9f2268 (provide a facility for \"delayed\" progress\n>>> reporting, 2007-04-20), the progress code has allowed\n>>> callers to skip showing progress if they have reached a\n>>> percentage-threshold of the total work before the delay\n>>> period passes.\n>>> \n>>> But since 8aade107dd (progress: simplify \"delayed\" progress\n>>> API, 2017-08-19), that parameter is not available to outside\n>>> callers (we always passed zero after that commit, though\n>>> that was corrected in the previous commit to \"100%\").\n>>> \n>>> Let's drop the threshold code, which never triggers in\n>>> any meaningful way.\n>>> \n>>> Signed-off-by: Jeff King <peff@peff.net>\n>>> ---\n>>> I tweaked your patch slightly to clean up the now-simplified\n>>> conditional.\n>> \n>> Your first patch (\"progress: set default delay threshold to 100%, not 0%\")\n>> as well as the modifications to this one look good to me. Feel free\n>> to add my \"Signed-off-by: Lars Schneider <larsxschneider@gmail.com>\".\n>> \n>> Thanks,\n>> Lars\n>> \n>> \n>> PS: How do you generate the commit references \"hash (first line, date)\"?\n>> Git log pretty print?\n> \n> $ git grep -A5 'Copy commit summary' Documentation/SubmittingPatches\n> Documentation/SubmittingPatches:151:The \"Copy commit summary\" command\n> of gitk can be used to obtain this\n> Documentation/SubmittingPatches-152-format, or this invocation of `git show`:\n> Documentation/SubmittingPatches-153-\n> Documentation/SubmittingPatches-154-....\n> Documentation/SubmittingPatches-155-    git show -s --date=short\n> --pretty='format:%h (\"%s\", %ad)' <commit>\n\nThanks :-)"}]}