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

> -USAGE='[help|start|bad|good|new|old|skip|next|reset|visualize|replay|log|run]'
> +USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'

I think this patch makes the whole series go in the right direction.

I wonder if you can skip the "we only support new/old if you are not
doing bog-standard bad/good" step and start from this "bisect terms"
one, though.

Then you do not even have to treat new/old any specially, and do not
even have to list them in the above list.

> @@ -79,9 +81,16 @@ bisect_start() {
>       orig_args=$(git rev-parse --sq-quote "$@")
>       bad_seen=0
>       eval=''
> -     # 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

I like this change very much; it removes the mysteriously misnamed
start-bad-good variable (because you do not really _care_ that
'start' was what implicitly decided that good/bad pair is the term
we use in this session; what you care is that the terms are already
known or not).

That is another reason why I think it would be a better organization
for the patch series to do without the intermediate "we now add new/old
as another hardcoded values on top of the traditional bad/good".

That is, I would think a reasonable progression of the series would
look more like these three steps:

 - preliminary clean-up steps (e.g. "correct 'mistook'");

 - use $name_new and $name_old throughout the code, giving them
   'bad' and 'good' as hardcoded values; finally

 - add 'bisect terms' support.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to