git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Cc: Junio C Hamano <gitster@pobox.com>,
	antoine.delaite@ensimag.grenoble-inp.fr,
	louis--alexandre.stuber@ensimag.grenoble-inp.fr,
	chriscool@tuxfamily.org, thomasxnguy@gmail.com,
	valentinduperray@gmail.com, git@vger.kernel.org
Subject: Re: [PATCH] bisect: revise manpage
Date: Fri, 26 Jun 2015 16:55:57 +0200	[thread overview]
Message-ID: <558D67FD.2000607@alum.mit.edu> (raw)
In-Reply-To: <vpqwpyqws53.fsf@anie.imag.fr>

On 06/26/2015 02:50 PM, Matthieu Moy wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> * Remove the "Look for a fix instead of a regression in the code"
>>   example, as (1) it was in the "git bisect run" section, but it
>>   doesn't use that command, and (2) I think this usage is adequately
>>   explained in the "Alternate terms" section.
> [...]
>> -* Look for a fix instead of a regression in the code
>> -+
>> -------------
>> -$ git bisect start
>> -$ git bisect new HEAD    # current commit is marked as new
>> -$ git bisect old HEAD~10 # the tenth commit from now is marked as old
>> -------------
>> -+
>> -Let's consider the last commit has a given property, and that we are looking
>> -for the commit which introduced this property. For each commit the bisection
>> -guide us to, we will test if the property is present. If it is we will mark
>> -the commit as new with 'git bisect new', otherwise we will mark it as old.
>> -At the end of the bisect session, the result will be the first new commit (e.g
>> -the first one with the property).
> 
> I disagree with this one: it's in the example section, not bisect run.

Oh yeah, you're right. I mistakenly thought that the examples section
was subsidiary to the `git bisect run` section.

> The other explanations are nice, but never show the full sequence of
> commands so I think an example to sum up does help.

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 added text elsewhere that retains the spirit of this
explanation.)

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, and a
little motivational text showing how the alternate names make the
commands clearer? As terms one could maybe use fast/slow and make the
example about benchmark speeds or something?

By the way, when I was revising the text two things occurred to me that
have probably been discussed to death elsewhere but let me mention them
anyway:

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.

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?

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

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu

  reply	other threads:[~2015-06-26 14:56 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-26 11:30 [PATCH] bisect: revise manpage Michael Haggerty
2015-06-26 12:44 ` Christian Couder
2015-06-26 13:00   ` Matthieu Moy
2015-06-26 13:15     ` Christian Couder
2015-06-26 14:58       ` Michael Haggerty
2015-06-26 15:28         ` Christian Couder
2015-06-26 16:30           ` Matthieu Moy
2015-06-26 17:47         ` Junio C Hamano
2015-06-26 20:22         ` [PATCH v10.1 3/7] Documentation/bisect: revise overall content Matthieu Moy
2015-06-26 12:50 ` [PATCH] bisect: revise manpage Matthieu Moy
2015-06-26 14:55   ` Michael Haggerty [this message]
2015-06-26 16:35     ` Matthieu Moy
2015-06-26 17:54     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=558D67FD.2000607@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=antoine.delaite@ensimag.grenoble-inp.fr \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=louis--alexandre.stuber@ensimag.grenoble-inp.fr \
    --cc=thomasxnguy@gmail.com \
    --cc=valentinduperray@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).