On Wed, Jun 10, 2015 at 5:24 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> 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. I don't think this would improve maintainability, at least not for me. The doc currently uses "good/bad" everywhere. For example the description is: This command uses git rev-list --bisect to help drive the binary search process to find which change introduced a bug, given an old "good" commit object name and a later "bad" commit object name. And this is logical if the default is "good/bad". If we use name_new and name_old in the code, we make it harder for us to see if the doc matches the code. And unless we rewrite a lot of them, the tests too will mostly be still using "good/bad" so it will make it harder to maintain them too. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html