Subject: Re: [PATCH v3 06/10] progress.c: call progress_interval() from progress_test_force_update()

From: Taylor Blau <[email-masked]>
Date: Fri, 22 Oct 2021 18:28:43 -0400

On Thu, Oct 14, 2021 at 12:28:22AM +0200, Ævar Arnfjörð Bjarmason wrote:
> Define the progress_test_force_update() function in terms of
> progress_interval(). For documentation purposes these two functions
> have the same body, but different names. Let's just define the test
> function by calling progress_interval() with SIGALRM ourselves.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]>
> ---
>  progress.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/progress.c b/progress.c
> index 893cb0fe56f..7fcc513717a 100644
> --- a/progress.c
> +++ b/progress.c
> @@ -216,7 +216,7 @@ static void progress_interval(int signum)
>   */
>  void progress_test_force_update(void)
>  {
> -	progress_update = 1;
> +	progress_interval(SIGALRM);
>  }

I agree that the pre- and post-image of this patch do the same thing.
(And not that it matters, but the value for 'signum' could have been
whatever you want, since progress_interval() ignores it).

Is there a reason to make this change other than to make clear that the
two do the same thing? It may be worth calling that out explicitly in
your patch message if so. You kind of do this, but I'd be as explicit
as:

  "To make it clear that progress_test_force_update() is a synonym for
  progress_interval(), define for the former in terms of a single call
  to the latter."

I think your second sentence says basically that, but it took me a few
attempts to discover.

Thanks,
Taylor