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

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr> writes:

> -	# start_bad_good is used to detect if we did a 
> -	# 'git bisect start bad_rev good_rev'
> -	start_bad_good=0
> +	# terms_defined is used to detect if we did a
> +	# 'git bisect start bad_rev good_rev' or if the user
> +	# defined his own terms with git bisect terms
> +	terms_defined=0

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)

(Whether this is a rethorical question is up to you ;-) )

> +	if test -s "$GIT_DIR/TERMS_DEFINED"
> +	then
> +		terms_defined=1
> +		get_terms
> +		rm -rf "$GIT_DIR/TERMS_DEFINED"

I don't understand why you need to delete this file. I did not review
thoroughly so there may be a reason, but you can help the reader with a
comment here.

> +bisect_terms () {
> +	test $# -eq 2 ||
> +	die "You need to give me at least two arguments"
> +
> +	if ! test -s "$GIT_DIR/BISECT_START"
> +	then
> +		echo $1 >"$GIT_DIR/BISECT_TERMS" &&
> +		echo $2 >>"$GIT_DIR/BISECT_TERMS" &&
> +		echo "1" > "$GIT_DIR/TERMS_DEFINED"

Space after >.

> @@ -574,7 +600,7 @@ case "$#" in
>  		git bisect -h ;;
>  	start)
>  		bisect_start "$@" ;;
> -	bad|good|new|old)
> +	bad|good|new|old|$NAME_BAD|$NAME_GOOD)

See my other message about quoting.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/