Re: [PATCH] bisect: revise manpage

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Jun 26, 2015, 16:35 UTC
Message-ID: <vpq381etoks.fsf@anie.imag.fr>
In-Reply-To: <558D67FD.2000607@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> I didn't like this example so much because (1) the code snippet is
> pretty trivial, and (2) the explanation afterwards is more of a general
> explanation of `git bisect` than a description of this particular
> example.

I agree that the explanations were redundant. I removed it.

> If you want to keep this example, how about making it a little bit more
> interesting? Perhaps use `git bisect terms` instead of new/old,

I now have both.

> and a little motivational text showing how the alternate names make
> the commands clearer?

Well, actually the motivational text would be essentially what was already said.

Show 6 quoted lines

> 1. I found it confusing that `git bisect terms` lists its arguments in
> the order `<term-new> <term-old>`. I think that listing them in
> "chronological" order would have been a lot more intuitive. But I expect
> this choice was made because `git bisect start` takes optional arguments
> in that order, so the inconsistency might be worse than the backwardness
> of this single command's arguments.

Yes, I think keeping the order of 'git bisect start' is good. Junio also mentionned alphabetic order (bad -> good, new -> old).

Show 5 quoted lines

> 2. When I was describing "old/new", I kept wishing that I could type
> "before/after" instead, because those terms seemed to agree better with
> the prose description of what "old/new" mean. I wonder if "before/after"
> might be better names for commits determined to be before/after the
> change being sought?

I like old/new essentially because they are very short. I would keep the code as-is for now, but it's very easy to add a before/after couple of terms later if needed. If others think before/after are better, it's still time to change it.

> Oh and I just noticed that `git bisect terms` is missing from the
> synopsis at the top of the man page.

Fixed.

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