Subject: Re: [PATCH v3 00/10] progress: assert "global_progress" + test fixes / cleanup

From: Emily Shaffer <[email-masked]>
Date: Thu, 21 Oct 2021 20:44:10 -0700

On Thu, Oct 14, 2021 at 12:28:16AM +0200, Ævar Arnfjörð Bjarmason wrote:
> 
> This series fixes various issues in and related to progress.c, and
> adds a BUG() assertion for us not starting two progress bars at the
> same time. Those changes are needed for subsequent changes that do
> more interesting things with this new global progress bar.
> 
> This v3 hopefully addresses all the feedback on the v2, thanks
> all. Changes:
> 
>  * Fix a memory leak in 1/10, and make the progress tests use the
>    SANITIZE=leak test mode.
> 
>  * Simplified some of the test-progress.c code (no more "start"
>    handling, the "total" count is mandatory now.
> 
>  * Split out a formatting change into 2/10 to make 3/10 easier to
>    read.
> 
>  * A new 9/10 makes an ad-hoc test recipie in 10/10 easier to explain
>    (in response to Emily's comment).
> 
>  * The BUG() assertion in 10/10 now has a much better message, we dump
>    the title of the two progress bars in play if we have a bug where
>    we started two at the same time.

One thing I noticed (and so did SZEDER) was the addition of string_list
to the helper; I made a comment accordingly in the patch where it was
added. It kind of seemed like an oversight - but I remember you were
wanting to head in that direction (multiple progress bar support) so
maybe it was paving the way for that?

Anyways, with or without it the series looks OK to me in this round too.

 - Emily