On Tue, Jun 9, 2015 at 9:01 AM, Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote: >> Subject: Re: [PATCH 3/4] bisect: simplify the add of new bisect terms > > s/add/addition/ > > Antoine Delaite <antoine.delaite@xxxxxxxxxxxxxxxxxxxxxxx> writes: > >> +static const char *name_bad; >> +static const char *name_good; > > Same remark as PATCH 2. As for patch 2, I think "name_bad" and "name_good" are better than "name_new" and "name_old". [...] >> + name_bad = "bad"; >> + name_good = "good"; >> + } else { >> + strbuf_getline(&str, fp, '\n'); >> + name_bad = strbuf_detach(&str, NULL); >> + strbuf_getline(&str, fp, '\n'); >> + name_good = strbuf_detach(&str, NULL); >> + } > > I would have kept just > > name_bad = "bad"; > name_good = "good"; > > in this patch, and introduce BISECT_TERMS in a separate one. Yeah I agree that it is more logical to have first a patch that does on bisect.c the same thing as patch 2 does on git-bisect.sh. For example the patch series could be for now: 1) bisect: typo fix 2) bisect: replace hardcoded "bad|good" with variables 3) git-bisect: replace hardcoded "bad|good" with variables 4) bisect: simplify adding new terms 5) bisect: add old/new terms -- 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