Subject: Re: [PATCH v3 05/11] commit-reach: start reporting errors in `paint_down_to_common()`

From: Junio C Hamano <[email-masked]>
Date: Tue, 27 Feb 2024 10:24:48 -0800

Johannes Schindelin <[email-masked]> writes:

> Is it lazy to omit the `< 0` here? Not actually, the reason why I omitted
> it here was to stay under 80 columns per line.
>
> Good eyes, though. If `paint_down_to_common()` _did_ return values other
> than -1 and 0, in particular positive ones that would not indicate a fatal
> error, the hunk under discussion would have introduced a problematic bug.

The same patch does compare the returned value with '< 0' in another
function (that is far from this place), which probably made the hunk
stand out during a review, I suspect.

After having fixed a bug elsewhere about a codepath that mixed the
"non-zero is an error" and "negative is an error" conventions during
the last cycle, I would have to say that being consistent would be
nice.  I think we at the end decided to make the callee be more
strict (returning negative to signal an error) while allowing the
caller to be more lenient (taking non-zero as an error) in that
case.

Thanks.