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

2 messages, 2 authors, 2016-07-27 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-07-26 17:34:16

Torsten Bögershausen [off-list ref] writes:
On 07/25/2016 06:53 PM, Junio C Hamano wrote:
quoted
Pranit Bauva [off-list ref] writes:
quoted
quoted
quoted
quoted
quoted
quoted
+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.

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

From: Pranit Bauva <hidden>
Date: 2016-07-27 04:20:55

Hey Junio,

On Tue, Jul 26, 2016 at 11:02 PM, Junio C Hamano [off-list ref] wrote:
Torsten Bögershausen [off-list ref] writes:
quoted
On 07/25/2016 06:53 PM, Junio C Hamano wrote:
quoted
Pranit Bauva [off-list ref] writes:
quoted
quoted
quoted
quoted
quoted
quoted
+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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help