Re: [PATCH v6 1/6] t0021: keep filter log files on comparison

From: Lars Schneider <larsxschneider@gmail.com>
Date: Jun 26, 2017, 09:02 UTC
Message-ID: <79C07E47-251D-4869-8EEA-11A9E336ABF3@gmail.com>
In-Reply-To: <xmqqtw337m4b.fsf@gitster.mtv.corp.google.com>

Show 20 quoted lines

> On 26 Jun 2017, at 00:12, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Lars Schneider <larsxschneider@gmail.com> writes:
> 
>> The filter log files are modified on comparison. Write the modified log files
>> to temp files for comparison to fix this.
> 
> The phrase "to fix this" implies that it is _wrong_ to modify after
> comparing it, but it is unclear _why_ you think is wrong.  After
> all, the purpose of this comparison helper is to see if these two
> are the same with cruft removed, and once the helper finds the
> answer to that question, the current users of the comparison helper
> do not reuse these files, so from _their_ point of view, there is
> nothing to "fix", is there?
> 
> It would become a problem _if_ we want future users of this helper
> to reuse the same expect (or actual) multiple times and start from
> an unmodified one.  There may be some other reason why you do not
> want the comparison to smudge these files.  Please state what that
> reason is before saying "fix this".

Understood. How about this?

    The filter log files are modified on comparison. That might be 
    unexpected by the caller. It would be even undesirable if the caller 
    wants to reuse the original log files.

    Address these issues by using temp files for modifications. This is 
    useful for the subsequent patch 'convert: add "status=delayed" to 
    filter process protocol'.

If this is OK, then do you want me to resend the series or can you fix it in place?

Thanks, Lars