Subject: Re: [PATCH v10 12/12] bisect--helper: `get_terms` & `bisect_terms` shell function in C

From: Junio C Hamano <[email-masked]>
Date: Wed, 27 Jul 2016 09:13:06 -0700

Pranit Bauva <[email-masked]> writes:

> On Tue, Jul 26, 2016 at 11:02 PM, Junio C Hamano <[email-masked]> wrote:
>> Torsten Bögershausen <[email-masked]> writes:
>>
>>> On 07/25/2016 06:53 PM, Junio C Hamano wrote:
>>>> Pranit Bauva <[email-masked]> writes:
>>>>
>>>>>>> >>> +enum terms_defined {
>>>>>>> >>> +       TERM_BAD = 1,
>>>>>>> >>> +       TERM_GOOD = 2,
>>>>>>> >>> +       TERM_NEW = 4,
>>>>>>> >>> +       TERM_OLD = 8
>>>>>>> >>> +};
>>>>>>> >>> +
>>>>>> >> ...
>>> Is there any risk that a more generic term like "TERM_BAD" may collide
>>> with some other definition some day ?
>>>
>>> Would it make sense to call it GIT_BISECT_TERM_BAD, GBS_TERM_BAD,
>>> BIS_TERM_BAD or something more unique ?
>>
>> I am not sure if the scope of these symbols would ever escape
>> outside bisect-helper.c (and builtin/bisect.c eventually when we
>> retire git-bisect.sh), but BISECT_TERM_{GOOD,BAD,OLD,NEW} would not
>> be too bad.
>
> I agree that it wouldn't be too bad. This can be considered as low
> hanging fruit and picked up after the completion of the project as
> after the whole conversion, some re-ordering of code would need to be
> done.
> For eg. there is read_bisect_terms() is in bisect.c while
> get_terms() is in builtin/bisect--helper.c but they both do the same
> stuff except the later one uses strbuf and a lot more important stuff.

The criteria to decide if a known "room for improvement" can be left
as a "can be later touched up" is "would it be a long-term
maintenance burden if it is left unfixed?", and from that point of
view, I actually view the latter as a part of the necessary
"polishing in response to review comments" for the initial version
of a new topic to land in the official project's tree.

As to TERM vs BISECT_TERM, I do not think it matters that much as I
said (IOW, I do not think it would make it a maintenance burden if
we kept using TERM_{BAD,GOOD,NEW,OLD}).