diff mbox series

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

Message ID patch-v3-06.10-c7c3843564e-20211013T222329Z-avarab@gmail.com (mailing list archive)
State Superseded
Headers show
Series progress: assert "global_progress" + test fixes / cleanup | expand

Commit Message

Ævar Arnfjörð Bjarmason Oct. 13, 2021, 10:28 p.m. UTC
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 <avarab@gmail.com>
---
 progress.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Taylor Blau Oct. 22, 2021, 10:28 p.m. UTC | #1
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 <avarab@gmail.com>
> ---
>  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
diff mbox series

Patch

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);
 }
 
 static void set_progress_signal(void)