Subject: Re: [PATCH 4/4] bisect: add the terms old/new

From: Junio C Hamano <[email-masked]>
Date: Wed, 10 Jun 2015 08:24:48 -0700

Matthieu Moy <[email-masked]> writes:

> "Somebody else did it like that" is not a good justification. Especially
> when the previous code was not merged: the code wasn't finished.
>
> But I actually disagree with the fact that it was not the idea. The
> point of having the terms in BISECT_TERMS was precisely to be generic
> enough. Had the goal been just to distinguish good/bad and old/new, we
> would have needed only one bit of information, and encoding it with the
> existance/non-existance of a file would have been sufficient (as you
> tried to do in addition to BISECT_TERMS).
>
>> For now we just rebased, corrected and finishing to implement
>> functionalities.
>
> functionalities is one thing, but the code should be maintainable to be
> merged in git.git. Git would not be where it is if Junio was merging
> patches based on "it works, we'll see if the code is good enough later"
> kinds of judgments ;-).
>
> Moving from "one hardcoded pair of terms" to "two hardcoded pairs of
> terms" is a nice feature, but hardly a step in the right direction wrt
> maintainability.

Nicely put.  From that point of view, the variable names and the
underlying machinery in general should call these two "new" vs
"old".  I.e. name_new=bad name_old=good would be the default, not
name_bad=bad name_good=good.