Re: [PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-10 Thread Louis-Alexandre Stuber
That is very different from ENOENT, which is an expected error when you are not using a customized terms. But in the current state, we are going to create bisect_terms even if the bisection is in bad/good mode. Should we differentiate the erors then ? and should we abort the bisection instead

Re: [PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-10 Thread Louis-Alexandre Stuber
But ENOENT is not a normal case at all. Should we not treat it the same way as other fopen() errors ? (either going on with default case or returning an error) Would : if (!fp) { die(could not read file '%s': %s, filename,

Re: [PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-10 Thread Matthieu Moy
Louis-Alexandre Stuber stub...@ensimag.grenoble-inp.fr writes: That is very different from ENOENT, which is an expected error when you are not using a customized terms. But in the current state, we are going to create bisect_terms even if the bisection is in bad/good mode. Which means that

[PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-10 Thread Antoine Delaite
Hi, Thanks for the review ! (sorry if you received this twice) Christian Couder christian.cou...@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'); +

Re: [PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-10 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: Louis-Alexandre Stuber stub...@ensimag.grenoble-inp.fr writes: That is very different from ENOENT, which is an expected error when you are not using a customized terms. But in the current state, we

Re: [PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-10 Thread Matthieu Moy
Louis-Alexandre Stuber stub...@ensimag.grenoble-inp.fr writes: But ENOENT is not a normal case at all. Should we not treat it the same way as other fopen() errors ? (either going on with default case or returning an error) Would : if (!fp) { die(could not read

Re: [PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-10 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: But I do not think it is a good idea to penalize the normal case by writing the terms file and reading them back from it when the user is bisecting with good/bad in the first place, so No strong

Re: [PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-10 Thread Matthieu Moy
Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr writes: +get_terms () { +if test -s $GIT_DIR/BISECT_TERMS +then +NAME_BAD=$(sed -n 1p $GIT_DIR/BISECT_TERMS) +NAME_GOOD=$(sed -n 2p $GIT_DIR/BISECT_TERMS) It is sad that we need to open

Re: [PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-10 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes: Louis-Alexandre Stuber stub...@ensimag.grenoble-inp.fr writes: That is very different from ENOENT, which is an expected error when you are not using a customized terms. But in the current state, we are going to create bisect_terms even if

Re: [PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-09 Thread Christian Couder
On Tue, Jun 9, 2015 at 9:01 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Subject: Re: [PATCH 3/4] bisect: simplify the add of new bisect terms s/add/addition/ Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr writes: +static const char *name_bad; +static const char *name_good

Re: [PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-09 Thread Matthieu Moy
Subject: Re: [PATCH 3/4] bisect: simplify the add of new bisect terms s/add/addition/ Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr writes: +static const char *name_bad; +static const char *name_good; Same remark as PATCH 2. } else if (starts_with(refname, good-)) { Did

Re: [PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-09 Thread Antoine Delaite
Hi, Thanks for the review, Junio C Hamano gits...@pobox.com writes: /* + * The terms used for this bisect session are stocked in + * BISECT_TERMS: it can be bad/good or new/old. + * We read them and stock them to adapt the messages + * accordingly. Default is bad/good. + */

Re: [PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-09 Thread Louis-Alexandre Stuber
Matthieu Moy matthieu@grenoble-inp.fr wrote: I think you would want to error out if errno is not ENOENT. Junio C Hamano jch2...@gmail.com wrote: We might want to see why fopen() failed here. If it is because the file did not exist, great. But otherwise? This file is always supposed to

Re: [PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-09 Thread Junio C Hamano
Louis-Alexandre Stuber stub...@ensimag.grenoble-inp.fr writes: Matthieu Moy matthieu@grenoble-inp.fr wrote: I think you would want to error out if errno is not ENOENT. Junio C Hamano jch2...@gmail.com wrote: We might want to see why fopen() failed here. If it is because the file did

Re: [PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-08 Thread Junio C Hamano
Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr writes: We create a file BISECT_TERMS in the repository .git to be read during a bisection. The fonctions to be changed if we add new terms are quite few. In git-bisect.sh : check_and_set_terms bisect_voc In bisect.c :

[PATCH 3/4] bisect: simplify the add of new bisect terms

2015-06-08 Thread Antoine Delaite
We create a file BISECT_TERMS in the repository .git to be read during a bisection. The fonctions to be changed if we add new terms are quite few. In git-bisect.sh : check_and_set_terms bisect_voc In bisect.c : handle_bad_merge_base Signed-off-by: Antoine Delaite