Re: [PATCH 1/2] format-patch: have progress option while generating patches

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> As I said above, I think I'd prefer it to require "--progress", as
> format-patch is quite often used as plumbing.

Yes, that sounds sensible.

Initially, my reaction was "Why do we even need --progress for
format-patch, when it gives one-line per patch output to show the
progress anyway?", but if that output is redirected to a file, of
course you'd need --progress independently.


> Should this use start_progress_delay()? In most cases the command will
> complete very quickly, and the progress report is just noise. For many
> commands (e.g., checkout) we wait 1-2 seconds before bothering to show
> progress output.

It is better to use the "delay" version for progress meters for
commands that may or may not last very long, and this may be a good
candidate to heed that principle.

The subcommands that use start_progress() tend to be older and more
batch oriented operations, e.g. fsck, pack-objects, etc., that are
expected to last longer.  It may be a good idea to convert them to
the "delay" variant, but obviously that is outside the scope of this
patch.

> I would have expected this to say "Generating patches"; most of our
> other progress messages are pluralized. You could use Q_() to handle the
> mono/plural case, but I think it's fine to just always say "patches"
> (that's what other messages do).
>
> One final thought is whether callers would want to customize this
> message, since it will often be used as plumbing. E.g., would rebase
> want to say something besides "Generating patches". I'm not sure.
> Anyway, if you're interested in that direction, there's prior art in
> the way rev-list handles "--progress" (and its sole caller,
> check_connected()).

These are all good suggestions.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]