Re: [PATCH v4 11/15] replay: use standard revision ranges

>
> Hi Christian,
>
> I am only reviewing the docs. To assume the mindset of a Git user
> unfamiliar with this command, I purposely did not read the cover letter
> until after this review was done.

> Christian Couder <christian.couder@gmail.com> writes:
>
> > [...]
> >
> > diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
> > new file mode 100644
> > index 0000000000..9a2087b01a
> > --- /dev/null
> > +++ b/Documentation/git-replay.txt
> > @@ -0,0 +1,90 @@
> > +git-replay(1)
> > +=============
> > +
> > +NAME
> > +----
> > +git-replay - Replay commits on a different base, without touching working tree
>
> How about using the same language ("new location") as in the DESCRIPTION
> heading?

> > +SYNOPSIS
> > +--------
> > +[verse]
> > +'git replay' --onto <newbase> <revision-range>...
> > +
> > +DESCRIPTION
> > +-----------
> > +
> > +Takes a range of commits, and replays them onto a new location.
>
> OK.
>
> > Does
> > +not touch the working tree or index, and does not update any
> > +references.
>
> How about this version?
>
>     The new commits are created without modifying the working tree,
>     index, or any references.

> > However, the output of this command is meant to be used
> > +as input to `git update-ref --stdin`, which would update the relevant
> > +branches.
>
> Before we get to this sentence, it would be great to explain why this
> command is useful (what problem does it solve)?
>
> Also, it would help to add "(see OUTPUT section below)" as a
> navigational aid in case some readers are wondering what the output
> looks like (and have not yet gotten to that section).
>
> I've noticed that you're using the phrase "starting point" in the
> OPTIONS section. I think this is better than "location" or "base" (FWIW
> we started using "starting point" in 0a02ca2383 (SubmittingPatches:
> simplify guidance for choosing a starting point, 2023-07-14)).
>
> The following is a version of this section that attempts to address my
> comments above (I've included the other bits already reviewed earlier to
> make it easier to read):
>
>     Takes a range of commits, and replays them onto a new starting
>     point. The new commits are created without modifying the working
>     tree, index, or any branches. If there are branches that point to
>     the commits in <revision-range>, list them in the output with both
>     the original commit hash and the corresponding (replayed) commit
>     hash (see OUTPUT section below) .

>     By combining this command with `git update-ref --stdin`, the
>     relevant branches can be updated. That is, the branches that were
>     pointing originally to the commits given in the <revision-range>
>     will be updated to point to the replayed commits. This is similar to
>     the way how `git rebase --update-refs` updates multiple branches in
>     the affected range.

> > +THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
> > +
> > +OPTIONS
> > +-------
> > +
> > +--onto <newbase>::
>
> How about 'starting-point' instead of 'newbase'?

> > +The update-ref command(s) in the output will update the branch(es) in
> > +the revision range to point at the new commits, similar to the way how
> > +`git rebase --update-refs` updates multiple branches in the affected
> > +range.
>
> Ah, good example. I've moved this to my larger example above, so I don't
> think this paragraph is needed here any more (it probably didn't belong
> in OPTIONS anyway).

> > +<revision-range>::
> > +     Range of commits to replay; see "Specifying Ranges" in
> > +     linkgit:git-rev-parse.
>
> OK.
>
> > +OUTPUT
> > +------
> > +
> > +When there are no conflicts, the output of this command is usable as
> > +input to `git update-ref --stdin`.
>
> What happens if there are conflicts? Probably good to mention in the
> DISCUSSION section. Some questions you may want to answer for the
> reader:
>
> (1) Is git-replay an all-or-nothing operation? That is, if there are any
> conflicts, is the output always empty (or do we still output those
> branches that *can* be updated without conflicts)?
>
> (2) What is meant by "conflict" for git-replay? Is it the same meaning
> as the case for git-rebase?
>
> For (1), in your cover letter under "# Important limitations" you say
> "No resumability" but I am not sure if this means git-replay will output
> *something* before exiting with an error, or simply nothing at all.

> > +     update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
> > +     update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
> > +     update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH}
> > +
> > +where the number of refs updated depends on the arguments passed and
> > +the shape of the history being replayed.
>
> Let's use "number of branches" instead of "number of refs" here to be
> consistent with the language elsewhere.

> > +EXIT STATUS
> > +-----------
> > +
> > +For a successful, non-conflicted replay, the exit status is 0.  When
> > +the replay has conflicts, the exit status is 1.
>
> OK.
>
> > If the replay is not
> > +able to complete (or start) due to some kind of error, the exit status
> > +is something other than 0 or 1.
>
> Not sure how useful "due to some kind of error" is here --- presumably
> the inability to replay is always due to some kind of error.

> > +EXAMPLES
> > +--------
> > +
> > +To simply rebase mybranch onto target:
>
> Looking at the CLI arguments, I think this phrase should be:
>
>     Replay the commits in `origin/main..mybranch` onto `target`:

> > +------------
> > +$ git replay --onto target origin/main..mybranch
> > +update refs/heads/mybranch ${NEW_mybranch_HASH} ${OLD_mybranch_HASH}
> > +------------
> > +
> > +When calling `git replay`, one does not need to specify a range of
> > +commits to replay using the syntax `A..B`; any range expression will
> > +do:
> > +
> > +------------
> > +$ git replay --onto origin/main ^base branch1 branch2 branch3
>
> Instead of `base`, how about `olderbranch`?

> > +update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
> > +update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
> > +update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH}
> > +------------
> > +
> > +This will simultaneously rebase branch1, branch2, and branch3 -- all
> > +commits they have since base, playing them on top of origin/main.
>
> How about
>
>     This will rebase the commits in `branch1`, `branch2`, and `branch3`
>     (excluding those in `base`), preplaying them on top of `origin/main`.
>
> > +These three branches may have commits on top of base that they have in
> > +common, but that does not need to be the case.
>
> s/base/`base`

> > +GIT
> > +---
> > +Part of the linkgit:git[1] suite
>
> One question I do have is what happens if you run git-replay twice
> (successfully)? Does the second invocation create another set of (new)
> successfully replayed commits?