Re: [PATCH v2 7/7] bisect: allows any terms set by user

2015-06-14 Thread Louis-Alexandre Stuber
Matthieu Moy matthieu@grenoble-inp.fr writes: Modifying in PATCH 7 some code that you introduced in PATCH 3 is suspicious. Is there any reason you did not name the variable terms_defined in the first place? (i.e. squash this hunk and the other instance of start_bad_good into PATCH 3)

Re: [PATCH v2 4/7] bisect: add the terms old/new

2015-06-14 Thread Louis-Alexandre Stuber
I agree this makes no sense hardcoding old/new once bisect terms is considerred. It would have been a lot better if we had started implementing bisect terms immediatly (but we thought old/new would already be an appreciable step for most of users). -- To unsubscribe from this list: send the line

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-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 1/2] git-bisect.sh : create a file if the bisection is in old/new mode, named BISECT_OLDNEWMODE, so it can easily be seen outside the program without having to read BISECT_TERMS. This wil

2015-06-07 Thread Louis-Alexandre Stuber
Thank you for the feedback. We are trying to apply all of your suggestions, but we would prefer to rebase the history before doing some of them (like renaming variables). About the BISECT_OLDNEWMODE file: The current implementation changes almost nothing to revision.c. We thought it was