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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 27, 2024, 18:24 UTC
Message-ID: <xmqqwmqpsr2n.fsf@gitster.g>
In-Reply-To: <fa8ebbfd-e01f-fbde-c851-54c162b610ff@gmx.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 6 quoted lines

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