git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Cc: Matthew Brett <matthew.brett@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Reuben Thomas <rrt@sc3d.org>
Subject: Re: Advice on edits to git-rebase man page
Date: Thu, 05 Feb 2015 10:58:21 -0800	[thread overview]
Message-ID: <xmqqa90smbhu.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <vpqa90s4oz2.fsf@anie.imag.fr> (Matthieu Moy's message of "Thu, 05 Feb 2015 11:44:33 +0100")

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

>   NAME
>        git-rebase - Forward-port local commits to the updated upstream head
>
> => Quite technical already.

Very much true, and I would say the description is "technically
correct" in the sense that "it is not wrong per-se".  There are a
few points that this fails to state and some that this overspecifies.

 - Rebase is about changing the base of an existing branch, but the
   description does not even mention the word "branch" [*1*].

 - There is nothing "forward" about it.  I often see myself applying
   (tentatively) incoming patches on top of whatever commit I happen
   to have checked out, review it and then rebasing it to apply to
   older maintenance track if the topic is about fixing an old bug.

 - There is no point stressing "local" commits; all the operations
   you do to munge commits are local.

Perhaps something like this instead?

    git-rebase - Rebuild a branch on top of a different commit


>   DESCRIPTION
>        If <branch> is specified, git rebase will perform an automatic
>        git checkout <branch> before doing anything else. Otherwise it
>        remains on the current branch.
>
> => Ouch, do we really want to start a documentation like this?

No.  We should say what the command does and what the command is for
in more general terms first and then describe how arguments can be
used to affect it.

> So, the DESCRIPTION part can definitely be improved IMHO. Your notation
> <graft-point>, <exclude-from> and <include-from> may be an improvement
> already.

<graft-point>, <exclude-from> and <include-from> aren't technically
wrong per-se, but I do not think bulk-replacing the words currently
used in the manual page with these is an improvement at all, unless
the mental picture the explanation draws is also updated to match
these new words.

reBASE is about changing the base of the affected branch, and the
mental picture the current documentation draws is "there is a plant,
from which you cut a branch, scion. Then you graft the scion onto
understock".  It calls the original tree (that the branch being
transplanted is part of) the "old-base", and the understock (that
the scion is going to be grafted onto) the "new-base".  The word
"graft" in "graft point" may better convey that we are doing a
transplanting than the current wording, but the word "point" makes
it unclear to the readers if it refers to the "point" where the
scion was cut from or where it is going to transplanted to, I am
afraid.

Also <exclude-from> and <include-from> is probably too operational,
and describing the command with only these two words would miss the
point that the command is about transplanting a branch.  It is true
that in order to transplant a branch, you first need to figure out
the set of commits whose effects are to be replayed, you would need
<exclude-from>..<include-from> range computation, but it is an
lower-level implemention detail.

> Some concrete examples may help too, like "I started developing against
> origin/foo, on local branch bar, and now want to rebase my work on top
> of origin/boz".

Very much so, but I wonder if it would be better to just refer to
examples in the tutorial (and if we do not have good examples there,
we should add some).

[Footnote]

 *1* Yes, "git rebase $newbase $commit^0" form can be used to
     transplant a part of history leading to the $commit that does
     not sit at the tip of a branch (nor repoint a branch to point
     at the result at the end) as if $commit^0 were at the tip of
     some branch, but the key word here is "as if".  The user is
     allowed to use the command pretending something that is not a
     branch is a branch, but what it tells us is that the command
     *is* about a branch.

  reply	other threads:[~2015-02-05 18:58 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-04 20:21 Advice on edits to git-rebase man page Matthew Brett
2015-02-05 10:44 ` Matthieu Moy
2015-02-05 18:58   ` Junio C Hamano [this message]
2015-02-05 21:20     ` Matthieu Moy
2015-02-05 21:29       ` Junio C Hamano
2015-02-18 18:59     ` Matthew Brett
2015-02-23 18:24       ` Matthew Brett

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=xmqqa90smbhu.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=git@vger.kernel.org \
    --cc=matthew.brett@gmail.com \
    --cc=rrt@sc3d.org \
    /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).