From: Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
Date: Wed, 10 Jun 2015 07:09:42 GMT
Subject: [PATCH 3/4] bisect: simplify the add of new bisect terms
Message-ID: <1975406107.331328.1433920182747.JavaMail.zimbra@ensimag.grenoble-inp.fr>
In-Reply-To: <1183699596.323718.1433875699237.JavaMail.zimbra@ensimag.grenoble-inp.fr>

Hi, 

Thanks for the review ! 
(sorry if you received this twice)

Christian Couder <christian.couder@gmail.com> wrote : 

>>> + 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 

For now we will keep name_bad and name_good as variables. 
About the patch series shouldn't I squash the commit 2) and 3) into one?