Subject: Re: [PATCH v4 05/11] scalar-diagnose: move functionality to common location

From: Junio C Hamano <[email-masked]>
Date: Fri, 12 Aug 2022 14:20:54 -0700

Victoria Dye <[email-masked]> writes:

>> Improved error reporting in 'create_diagnostics_archive()'. I was
>> originally going to modify the "failed to write archive" error to trigger
>> whenever 'create_diagnostics_archive()' returned a nonzero value.
>> However, while working on it I realized the message would no longer be
>> tied to a failure of 'write_archive()', making it less helpful in
>> pinpointing an issue. To address the original issue
>> ('add_directory_to_archiver()' silently failing in
>> 'create_diagnostics_archive()'), I instead refactored those calls into a
>> loop and added the error message. Now, there's exactly one error message
>> printed for each possible early exit scenario from
>> 'create_diagnostics_archive()', hopefully avoiding both redundancy &
>> under-reporting.

Ah, I see.  I probably should have read the cover letter before
responding.  I try to understand the new iteration _without_ relying
on the cover letter first, to ensure that the resulting history is
still understandable; when I see something questionable, however, I
should see if cover letter gives more context and clues.  Sorry for
the noise.