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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 12, 2022, 21:20 UTC
Message-ID: <xmqqk07d9n0p.fsf@gitster.g>
In-Reply-To: <a728c894-4e4a-44c3-a1f5-f87c8bfae13a@github.com>

Victoria Dye <vdye@github.com> writes:

Show 12 quoted lines

>> 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.