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

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

 



Hey Junio,

On Tue, Jul 26, 2016 at 11:02 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Torsten Bögershausen <tboegi@xxxxxx> writes:
>
>> On 07/25/2016 06:53 PM, Junio C Hamano wrote:
>>> Pranit Bauva <pranit.bauva@xxxxxxxxx> 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.
Maybe after the whole conversion, the above enum (or #define bitmap)
should also be moved to bisect.h and be used consistently in bisect.c
too.

Regards,
Pranit Bauva
--
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]