Re: [PATCH v8 00/14] Introduce new `git replay` command

From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Christian Couder <christian.couder@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	 Patrick Steinhardt <ps@pks.im>, Elijah Newren <newren@gmail.com>,
	 John Cai <johncai86@gmail.com>,
	Derrick Stolee <stolee@gmail.com>,
	 Phillip Wood <phillip.wood123@gmail.com>,
	 Calvin Wan <calvinwan@google.com>, Toon Claes <toon@iotcl.com>,
	 Dragan Simic <dsimic@manjaro.org>,
	Linus Arver <linusa@google.com>
Subject: Re: [PATCH v8 00/14] Introduce new `git replay` command
Date: Sat, 25 Nov 2023 01:02:20 +0100 (CET)	[thread overview]
Message-ID: <6bfe1541-54dd-ca6b-e930-94d3038060f1@gmx.de> (raw)
In-Reply-To: <20231124111044.3426007-1-christian.couder@gmail.com>

Hi Christian,

On Fri, 24 Nov 2023, Christian Couder wrote:

> # Changes between v7 and v8
>
> Thanks to Dscho, Linus Arver, Dragan Simic, Elijah, Junio, Derrick
> Stolee, Phillip Wood, Calvin Wan and Toon Claes for their suggestions
> on the previous versions! The only few changes compared to v7 are:
>
> * The patch series was rebased onto master at 564d0252ca (Git 2.43,
>   2023-11-20). This is to make it stand on a stable base.
>
> * In patch 2/14 (replay: introduce new builtin), there is a synopsys
>   change in the doc, and the corresponding usage message change as
>   suggested by Dscho. This is just about replacing " # EXPERIMENTAL"
>   at the end of both the synopsys and usage message with
>   "(EXPERIMENTAL!) " at the beginning of them.

Thank you so much for going all the way to where we are now.

> CI tests seem to pass according to:
>
> https://github.com/chriscool/git/actions/runs/6979770154
>
> (Sorry I am not waiting more than 20 minutes for the 3 last ones to
> finish.)

Yes, our test suite takes too long, and I fear that there are quite a few
developers ignoring it as a consequence.

> # Range-diff between v7 and v8
>
> (A single change was made in patch 2/14, but unfortunately as the
> lines changed in that patch are also changed by other patches later,
> it looks like there are more changes in subsequent patches.)

Right, the lines added in 2/14 are changed multiple times over the course
of the patch series.

>  1:  cddcd967b2 =  1:  18fd9b0d5d t6429: remove switching aspects of fast-rebase
>  2:  c8476fb093 !  2:  fc6bdf4de4 replay: introduce new builtin
>     @@ Documentation/git-replay.txt (new)
>      +SYNOPSIS
>      +--------
>      +[verse]
>     -+'git replay' --onto <newbase> <oldbase> <branch> # EXPERIMENTAL
>     ++(EXPERIMENTAL!) 'git replay' --onto <newbase> <oldbase> <branch>
>      +
>      +DESCRIPTION
>      +-----------
>     @@ builtin/replay.c: int cmd__fast_rebase(int argc, const char **argv)
>      -
>         if (argc == 2 && !strcmp(argv[1], "-h")) {
>      -          printf("Sorry, I am not a psychiatrist; I can not give you the help you need.  Oh, you meant usage...\n");
>     -+          printf("git replay --onto <newbase> <oldbase> <branch> # EXPERIMENTAL\n");
>     ++          printf("usage: (EXPERIMENTAL!) git replay --onto <newbase> <oldbase> <branch>\n");
>                 exit(129);
>         }
>
>  3:  43322abd1e !  3:  e96a66c352 replay: start using parse_options API
>     @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
>         int ret = 0;
>
>      -  if (argc == 2 && !strcmp(argv[1], "-h")) {
>     --          printf("git replay --onto <newbase> <oldbase> <branch> # EXPERIMENTAL\n");
>     +-          printf("usage: (EXPERIMENTAL!) git replay --onto <newbase> <oldbase> <branch>\n");
>      -          exit(129);
>      +  const char * const replay_usage[] = {
>     -+          N_("git replay --onto <newbase> <oldbase> <branch> # EXPERIMENTAL"),
>     ++          N_("(EXPERIMENTAL!) git replay --onto <newbase> <oldbase> <branch>"),
>      +          NULL
>      +  };
>      +  struct option replay_options[] = {
>  4:  6524c7f045 =  4:  f819d283d9 replay: die() instead of failing assert()
>  5:  05d0efa3cb =  5:  68bbcf9492 replay: introduce pick_regular_commit()
>  6:  c7a5aad3d6 =  6:  72221c647e replay: change rev walking options
>  7:  01f35f924b =  7:  f54d8fce22 replay: add an important FIXME comment about gpg signing
>  8:  1498b24bad =  8:  e50cc22522 replay: remove progress and info output
>  9:  6786fc147b =  9:  0c5ea3d18e replay: remove HEAD related sanity check
> 10:  9a24dbb530 = 10:  9fc636fc3d replay: make it a minimal server side command
> 11:  ad6ca2fbef ! 11:  2096bcad79 replay: use standard revision ranges
>     @@ Documentation/git-replay.txt: git-replay - EXPERIMENTAL: Replay commits on a new
>       SYNOPSIS
>       --------
>       [verse]
>     --'git replay' --onto <newbase> <oldbase> <branch> # EXPERIMENTAL
>     -+'git replay' --onto <newbase> <revision-range>... # EXPERIMENTAL
>     +-(EXPERIMENTAL!) 'git replay' --onto <newbase> <oldbase> <branch>
>     ++(EXPERIMENTAL!) 'git replay' --onto <newbase> <revision-range>...
>
>       DESCRIPTION
>       -----------
>     @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
>         int ret = 0;
>
>         const char * const replay_usage[] = {
>     --          N_("git replay --onto <newbase> <oldbase> <branch> # EXPERIMENTAL"),
>     -+          N_("git replay --onto <newbase> <revision-range>... # EXPERIMENTAL"),
>     +-          N_("(EXPERIMENTAL!) git replay --onto <newbase> <oldbase> <branch>"),
>     ++          N_("(EXPERIMENTAL!) git replay --onto <newbase> <revision-range>..."),
>                 NULL
>         };
>         struct option replay_options[] = {
> 12:  081864ed5f ! 12:  d5414806ef replay: add --advance or 'cherry-pick' mode
>     @@ Documentation/git-replay.txt: git-replay - EXPERIMENTAL: Replay commits on a new
>       SYNOPSIS
>       --------
>       [verse]
>     --'git replay' --onto <newbase> <revision-range>... # EXPERIMENTAL
>     -+'git replay' (--onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL
>     +-(EXPERIMENTAL!) 'git replay' --onto <newbase> <revision-range>...
>     ++(EXPERIMENTAL!) 'git replay' (--onto <newbase> | --advance <branch>) <revision-range>...
>
>       DESCRIPTION
>       -----------
>     @@ builtin/replay.c: static struct commit *pick_regular_commit(struct commit *pickm
>         int ret = 0;
>
>         const char * const replay_usage[] = {
>     --          N_("git replay --onto <newbase> <revision-range>... # EXPERIMENTAL"),
>     -+          N_("git replay (--onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL"),
>     +-          N_("(EXPERIMENTAL!) git replay --onto <newbase> <revision-range>..."),
>     ++          N_("(EXPERIMENTAL!) git replay (--onto <newbase> | --advance <branch>) <revision-range>..."),
>                 NULL
>         };
>         struct option replay_options[] = {
> 13:  19c4016c7c ! 13:  2a3e521c13 replay: add --contained to rebase contained branches
>     @@ Documentation/git-replay.txt: git-replay - EXPERIMENTAL: Replay commits on a new
>       SYNOPSIS
>       --------
>       [verse]
>     --'git replay' (--onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL
>     -+'git replay' ([--contained] --onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL
>     +-(EXPERIMENTAL!) 'git replay' (--onto <newbase> | --advance <branch>) <revision-range>...
>     ++(EXPERIMENTAL!) 'git replay' ([--contained] --onto <newbase> | --advance <branch>) <revision-range>...
>
>       DESCRIPTION
>       -----------
>     @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
>         int ret = 0;
>
>         const char * const replay_usage[] = {
>     --          N_("git replay (--onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL"),
>     -+          N_("git replay ([--contained] --onto <newbase> | --advance <branch>) "
>     -+             "<revision-range>... # EXPERIMENTAL"),
>     +-          N_("(EXPERIMENTAL!) git replay (--onto <newbase> | --advance <branch>) <revision-range>..."),
>     ++          N_("(EXPERIMENTAL!) git replay "
>     ++             "([--contained] --onto <newbase> | --advance <branch>) "
>     ++             "<revision-range>..."),
>                 NULL
>         };
>         struct option replay_options[] = {
> 14:  29556bcc86 = 14:  93e034faee replay: stop assuming replayed branches do not diverge

The range-diff looks excellent!

Thank you for addressing all of my concerns, I am very much in favor of
getting this version into git/git's main branch.

Thank you,
Johannes