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

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Jun 11, 2015, 09:22 UTC
Message-ID: <vpqoakmoaz7.fsf@anie.imag.fr>
In-Reply-To: <xmqqk2vbi7rf.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 10 quoted lines

> Antoine Delaite <antoine.delaite@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.

While I think we should not hardcode too much in the code, I also think it makes sense to have hardcoded old/new in the user-interface. If you give me just

  git bisect terms <first-term> <second-term>

then half of the times, I'll use old/new in the wrong order. And if I hadn't looked to bisect close enough, I'd even complain that the terms are not usable interchangeably.

At least, a in a flow like

  git bisect start
  git bisect new
  git checkout <old-sha>
  git bisect old

there's no ambiguity.

So, to me, having both hardcoded old/new (unambiguous) and "git bisect terms" (for flexibility) makes sense.

Another important parameter: the students are finishing their project tomorrow. They may continue on their personnal free time, but at best their time budget is considerably reduced, and from my past experience, the time budget usually reaches 0. That's not a reason to merge bad code in git.git, but an incentive to close the series with something going in the right direction. Typically:

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

This is straightforward

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

This is relatively uncontroversial, and should be easy to finish. I would consider it a good thing anyway.

>  - add 'bisect terms' support.

I'd put this on the low-priority whishlist. If the codebase is ready and a preliminary patch exists, it will be relatively easy for someone else to finish the job.

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