Subject: Re: [PATCH v3 14/15] replay: add --contained to rebase contained branches

From: Toon Claes <[email-masked]>
Date: Thu, 22 Jun 2023 12:10:51 +0200

Christian Couder <[email-masked]> writes:

> diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
> index 439b2f92e7..6fcaa44ef2 100644
> --- a/Documentation/git-replay.txt
> +++ b/Documentation/git-replay.txt
> @@ -9,7 +9,7 @@ git-replay - Replay commits on a different base, without touching working tree
>  SYNOPSIS
>  --------
>  [verse]
> -'git replay' (--onto <newbase> | --advance <branch>) <revision-range>...
> +'git replay' ([--contained] --onto <newbase> | --advance <branch>)
> <revision-range>...

I'm not sure we need this, or at least not right now.
I've been testing with a repo having:

* a13d9c4 (another-feature) yet another commit
* c7afc2e (HEAD -> feature) third commit
* e95cecc second commit
* f141e77 first commit
| * 7bb62ac (main) later commit
| * 506cb0a another commit
|/
* e7acac6 initial commit

I tried both commands:

$ git replay --onto main main..feature main..another-feature
$ git replay --onto main --contained main..another-feature

and they both give the same result (especially with the commit following
up this one). What is the upside of having this --contained option?

Maybe it's better to defer this patch to a separate series.

And another question, in git-rebase(1) a similar option is called
--update-refs. Would you consider reusing that name here is a good idea
for consistency?

> diff --git a/builtin/replay.c b/builtin/replay.c
> index 4b64d15159..9b182eaa90 100644
> --- a/builtin/replay.c
> +++ b/builtin/replay.c
> @@ -303,6 +306,10 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
>  		usage_with_options(replay_usage, replay_options);
>  	}
>
> +	if (advance_name && contained)
> +		die(_("options '%s' and '%s' cannot be used together"),
> +		    "--advance", "--contained");

Is there a technical issue why we don't support this? I don't really see
why we couldn't.

--
Toon