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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]