Re: [RFC PATCH 02/10] range-diff.c: don't use st_mult() for signed "int"

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Dec 11, 2021, 14:01 UTC
Message-ID: <nycvar.QRO.7.76.6.2112111453040.90@tvgsbejvaqbjf.bet>
In-Reply-To: <211210.868rwscxcw.gmgdl@evledraar.gmail.com>

Hi Ævar,

On Fri, 10 Dec 2021, Ævar Arnfjörð Bjarmason wrote:

> But I'll happily admit ignorance on how the actual guts of range-diff
> work, I just wanted to fix a segfault I kept running into locally at
> some point, and figured I'd submit this RFC.

I understand that it is super tempting to avoid spending the time to understand how range-diff works and simply make changes until the segmentation fault is gone, and then shoot off several iterations of the patch series in the hopes that it gets merged at some point, and that maybe reviewers who do spend the time to become familiar with the logic help avoid introduce new bugs.

However, as a reviewer I am totally unsympathetic of this approach. I do not want to review patches that even go so far as renaming functions when they claim to "just fix a segfault" and the author even admits that they're unfamiliar with what the code is supposed to do, without any indication that they're inclined to invest the effort to change that.

If all you want to do is to fix the segmentation fault, and want to skip the due diligence of studying the business logic, then just fix that segmentation fault (I strongly suspect that using `COST()` after modifying it to use `st_*()` would accomplish that). No need to inflate that to 5 patches. Unless you're thinking of the commit-per-author count as some sort of scoreboard where you want to win. In which case I am even less interested in reviewing the patches.

Ciao, Johannes