> On 26 Jun 2017, at 19:50, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Lars Schneider <larsxschneider@xxxxxxxxx> writes:
>
>>> On 26 Jun 2017, at 00:17, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>>
>>> Lars Schneider <larsxschneider@xxxxxxxxx> writes:
>>>
>>>> "rot13-filter.pl" used to write "OUT <size>" to the debug log even in case of
>>>> an abort or error. Fix this by writing "OUT <size>" to the debug log only in
>>>> the successful case if output is actually written.
>>>
>>> Again, use of "Fix this" without clarifying what the problem is. Is
>>> this change needed because the size may not be known when the new
>>> filter protocol is in use, or something?
>>
>> How about this?
>>
>> "rot13-filter.pl" always writes "OUT <size>" to the debug log at the end
>> of an interaction.
>>
>> This works without issues for the existing cases "abort", "error", and
>> "success". In a subsequent patch 'convert: add "status=delayed" to
>> filter process protocol' we will add a new case "delayed". In that case
>> we do not send the data right away and it would be wrong/misleading to
>> the reader if we would write "OUT <size>" to the debug log.
>
> I still do not quite get "we do not send the data right away" as
> opposed to "at the end of an interaction" makes it wrong/misleading
> to write "OUT <size>"?
>
> A new response "delayed" that will be introduced in subsequent
> patches accepts the input, without giving the filtered result
> right away, and at that moment, we do not even have the output
> size yet.
>
> might be a very reasonable rationale for omitting "OUT: <size>" in
> the output when "delayed" request is seen, but that still does not
> explain why it is sensible to drop "OUT: <size>" for error or abort
> case.
Well, "rot13-filter.pl" *does* have the output available right away
even in the delayed case (because of the mock implementation).
If we print "OUT: <size>" all the time then this would lead to
misleading log messages in this situation.
It's not necessary to drop "OUT: <size>" for abort and error. It
was just the way that required the least number of lines of code.
> Having said all that, I tend to agree with the actual change. When
> we abort or error, we aren't producing any useful output, so it may
> be sensible _not_ to say "OUT: 0" in the log output from the test
> helper in these cases. And if the change is explained that way,
> readers would understand why this step is a good thing to have, with
> or without the future "delayed" step in the series.
How about this?
"rot13-filter.pl" always writes "OUT <size>" to the debug log at the end
of a response.
This works without issues for the existing responses "abort", "error",
and "success". A new response "delayed", that will be introduced in
subsequent patches, accepts the input without giving the filtered result
right away. Since we actually have the data already available in our mock
filter the debug log output would be wrong/misleading. Therefore, we
do not write "OUT <size>" for "delayed" responses.
To simplify the code we do not write "OUT <size>" for "abort" and
"error" responses, too, as their size is always zero.
- Lars
