git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] clone: fix progress-regression
@ 2012-05-07 19:23 Erik Faye-Lund
  2012-05-07 19:47 ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: Erik Faye-Lund @ 2012-05-07 19:23 UTC (permalink / raw)
  To: git; +Cc: gitster, j.sixt, peff, rctay89

In 5bd631b3 ("clone: support multiple levels of verbosity"), the
default behavior to show progress of the implicit checkout in
the clone-command regressed so that progress was only shown if
the verbose-option was specified.

Fix this by making option_verbosity == 0 output progress as well.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---

Hmpf. While tidying this up for a proper submission, I found out that
writing a reliable test is tricky due to the start_progress_delay-usage.
I'd have to clone a repo that does not progress more than 50% of the
checkout-operation within one second or something like that (I didn't
quite follow the logic there), which is a lot :P. This strikes me as
error-prone, and I can't see an obvious hack around it (other than
modifying the progress-code, which I consider a somewhat nasty
alternative).

So perhaps writing a test to avoid similar breakages in the future isn't
worth it?

 builtin/clone.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index bbd5c96..a4d8d25 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -569,7 +569,7 @@ static int checkout(void)
 	opts.update = 1;
 	opts.merge = 1;
 	opts.fn = oneway_merge;
-	opts.verbose_update = (option_verbosity > 0);
+	opts.verbose_update = (option_verbosity >= 0);
 	opts.src_index = &the_index;
 	opts.dst_index = &the_index;
 
-- 
1.7.10.msysgit.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] clone: fix progress-regression
  2012-05-07 19:23 [PATCH] clone: fix progress-regression Erik Faye-Lund
@ 2012-05-07 19:47 ` Junio C Hamano
  2012-05-08  5:46   ` Johannes Sixt
  0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2012-05-07 19:47 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: git, gitster, j.sixt, peff, rctay89

Erik Faye-Lund <kusmabite@gmail.com> writes:

> In 5bd631b3 ("clone: support multiple levels of verbosity"), the
> default behavior to show progress of the implicit checkout in
> the clone-command regressed so that progress was only shown if
> the verbose-option was specified.
>
> Fix this by making option_verbosity == 0 output progress as well.
>
> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> ---

Thanks.

> Hmpf. While tidying this up for a proper submission, I found out that
> writing a reliable test is tricky due to the start_progress_delay-usage.
> I'd have to clone a repo that does not progress more than 50% of the
> checkout-operation within one second or something like that (I didn't
> quite follow the logic there), which is a lot :P.

In real life, most of the checkout is fairly quick for even projects like
the kernel, and the logic is an attempt to avoid cluttering the terminal
with progress unless it takes unusually long and might make the user feel
worried.

> So perhaps writing a test to avoid similar breakages in the future isn't
> worth it?

I agree that it probably isn't worth it, given that it took forever for
anybody to even complain about this.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] clone: fix progress-regression
  2012-05-07 19:47 ` Junio C Hamano
@ 2012-05-08  5:46   ` Johannes Sixt
  0 siblings, 0 replies; 3+ messages in thread
From: Johannes Sixt @ 2012-05-08  5:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Erik Faye-Lund, git, peff, rctay89

Am 5/7/2012 21:47, schrieb Junio C Hamano:
> In real life, most of the checkout is fairly quick for even projects like
> the kernel, and the logic is an attempt to avoid cluttering the terminal
> with progress unless it takes unusually long and might make the user feel
> worried.

What is "unusually long"? On Windows, it can take its time to switch a
branch even if only a few dozen files are affected, the disk cache is
cold, and an on-access virus scanner is active. :-)

I appreciate the checkout progress indicator as eye candy every once in a
while.

-- Hannes

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-05-08  5:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-07 19:23 [PATCH] clone: fix progress-regression Erik Faye-Lund
2012-05-07 19:47 ` Junio C Hamano
2012-05-08  5:46   ` Johannes Sixt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).