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

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