Re: [PATCH v4 11/12] convert: add filter.<driver>.process option

> On 03 Aug 2016, at 23:43, Junio C Hamano <gitster@pobox.com> wrote:
> 
> On Wed, Aug 3, 2016 at 2:37 PM, Lars Schneider <larsxschneider@gmail.com> wrote:
>>> 
>>> I think this was already pointed out in the previous review by Peff,
>>> but a variable "ret" that says "0 is bad" somehow makes it hard to
>>> follow the code.  Perhaps rename it to "int error", flip the meaning,
>>> and if the caller wants this function to return non-zero on success
>>> flip the polarity in the return statement itself, i.e. "return !errors",
>>> may make it easier to follow?
>> 
>> This follows the existing filter function. Please see Peff's later
>> reply here:
> 
> Which I did before mentioning "pointed out in his review".
> 
>> That's why I kept it the way it is. If you prefer the "!errors" approach
>> then I will change that.
> 
> I am not suggesting to change the RETURN VALUE from this function.
> That is why I mentioned "return !errors" to flip the polarity at the end.
> Inside the function, "ret" variable _forces_ the readers to think "this
> function unlike the others signal an error with 0" constantly while
> reading it, and one possible approach to reduce the mental burden
> is to replace "ret" variable with "errors" variable, which is clear to
> anybody that it would be non-zero when we saw error(s).
> 
> Oh, I am not suggesting to _count_ the number of errors by
> mentioning a possible variable name "errors"; the only reason
> why I mentioned that name is because "error" is already
> taken, and "seen_error" is a bit too long.