Re: [PATCH v3 14/15] replay: add --contained to rebase contained branches
- From
Toon Claes <toon@iotcl.com>
- Date
- Jun 22, 2023, 10:10 UTC
- Message-ID
- <87cz1nst1s.fsf@iotcl.com>
- In-Reply-To
- <20230602102533.876905-15-christian.couder@gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
Show 11 quoted lines
> 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?
Show 11 quoted lines
> 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