From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Christian Couder <christian.couder@gmail.com>
Cc: Elijah Newren <newren@gmail.com>,
Derrick Stolee <derrickstolee@github.com>,
git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
Patrick Steinhardt <ps@pks.im>, John Cai <johncai86@gmail.com>,
Christian Couder <chriscool@tuxfamily.org>
Subject: Re: [PATCH 11/14] replay: use standard revision ranges
Date: Thu, 7 Sep 2023 12:22:45 +0200 (CEST) [thread overview]
Message-ID: <4a648214-2627-c9c0-0be2-5a694e8c3b3c@gmx.de> (raw)
In-Reply-To: <CAP8UFD2KMikjJJQ0FGXO1EYYHRNWHH2n70UATU_o61PJ411f2Q@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4502 bytes --]
Hi Christian,
On Thu, 7 Sep 2023, Christian Couder wrote:
> On Sun, Sep 3, 2023 at 5:47 PM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Sat, 29 Apr 2023, Elijah Newren wrote:
> >
> > > On Mon, Apr 24, 2023 at 8:23 AM Derrick Stolee <derrickstolee@github.com> wrote:
>
> > > > Basically, I'm not super thrilled about exposing options that are
> > > > unlikely to be valuable to users and instead are more likely to cause
> > > > confusion due to changes that won't successfully apply.
> > >
> > > Oh, I got thrown by the "right now" portion of your comment; I
> > > couldn't see how time or future changes would affect anything to make
> > > it less (or more) confusing for users.
> > >
> > > Quick clarification, though: while you correctly point out the type of
> > > confusion the user would experience without my overriding, my
> > > overriding of rev.reverse (after setup_revisions() returns, not before
> > > it is called) precludes that experience. The override means none of
> > > the above happens, and they would instead just wonder why their option
> > > is being ignored.
> >
> > FWIW here is my view on the matter: `git replay`, at least in its current
> > incarnation, is a really low-level tool. As such, I actually do not want
> > to worry much about protecting users from nonsensical invocations.
> >
> > In that light, I would like to see that code rejecting all revision
> > options except `--diff-algorithm` be dropped. Should we ever decide to add
> > a non-low-level mode to `git replay`, we can easily add some user-friendly
> > sanity check of the options then, and only for that non-low-level code.
> > For now, I feel that it's just complicating things, and `git replay` is in
> > the experimental phase anyway.
>
> I would be Ok with removing the patch (called "replay: disallow
> revision specific options and pathspecs")
> that rejects all revision options and pathspecs if there is a
> consensus for that.
Well, since you talk about "consensus" and I already made my strong
preference known, how about you add yours?
> It might not simplify things too much if there is still an exception for
> `--diff-algorithm` though. Also it's not clear if you are Ok with
> allowing pathspecs or not.
I want to remove all the rev-list-option-disallowing code. Including the
pathspec one.
> The idea with disallowing all of them was to later add back those that
> make sense along with tests and maybe docs to explain them in the
> context of this command. It was not to disallow them permanently. So I
> would think the best path forward would be a patch series on top of this
> one that would revert the patch disallowing these options and maybe
> pathspecs, and instead allow most of them and document and test things a
> bit.
I understand that that was the reasoning.
What I would like to suggest is that we should treat `git replay` as a
low-level (or: "plumbing" in Git Speak) command.
That would allow us to leave the option for stricter command-line
parameter validation to an _additional_ option, to be added later (or
never), and to stop worrying about this tool being used in ways that make
no sense (or make no sense _to us_, _now_).
> > And further, I would even like to see that `--reverse` override go, and
> > turn it into `revs.reverse = !revs.reverse` instead. (And yes, I can
> > easily think of instances where I would have wanted to reverse a series of
> > patches...).
>
> I think this might deserve docs and tests too,
I agree.
> so it might want to be part of a separate patch series once the existing
> one has graduated.
Here, I disagree. This is a bug, from my perspective, and needs to be
fixed before graduating.
> At this point I don't think it's worth delaying this patch series for
> relatively small issues like this. There are many different ways this
> new command can be polished and improved. The important thing is that
> it looks like we all agree that the new command makes sense and should
> have roughly the basic set of features that Elijah originally
> implemented, so let's go with this, and then we can improve and
> iterate on top of this.
If it were for a small issue such as a typo, sure.
But it is a bug that `--no-reverse` is hard-coded (in a confusing manner
so because the `reverse` attribute is set), and the
`--no-reverse`/`--reverse` options are ignored, silently so.
Ciao,
Johannes
next prev parent reply other threads:[~2023-09-07 15:24 UTC|newest]
Thread overview: 208+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-07 7:24 [PATCH 00/14] Introduce new `git replay` command Christian Couder
2023-04-07 7:24 ` [PATCH 01/14] replay: introduce new builtin Christian Couder
2023-04-07 7:24 ` [PATCH 02/14] replay: start using parse_options API Christian Couder
2023-04-07 7:24 ` [PATCH 03/14] replay: die() instead of failing assert() Christian Couder
2023-04-07 7:24 ` [PATCH 04/14] replay: introduce pick_regular_commit() Christian Couder
2023-04-07 7:24 ` [PATCH 05/14] replay: don't simplify history Christian Couder
2023-04-07 7:24 ` [PATCH 06/14] replay: add an important FIXME comment about gpg signing Christian Couder
2023-04-07 7:24 ` [PATCH 07/14] replay: remove progress and info output Christian Couder
2023-04-07 7:24 ` [PATCH 08/14] replay: remove HEAD related sanity check Christian Couder
2023-04-07 7:24 ` [PATCH 09/14] replay: very coarse worktree updating Christian Couder
2023-04-07 7:24 ` [PATCH 10/14] replay: make it a minimal server side command Christian Couder
2023-04-07 7:24 ` [PATCH 11/14] replay: use standard revision ranges Christian Couder
2023-04-14 14:09 ` Derrick Stolee
2023-04-14 14:23 ` Derrick Stolee
2023-04-15 19:07 ` Elijah Newren
2023-04-16 5:28 ` Elijah Newren
2023-04-17 14:05 ` Derrick Stolee
2023-04-18 5:54 ` Elijah Newren
2023-04-18 13:10 ` Derrick Stolee
2023-04-20 4:53 ` Elijah Newren
2023-04-20 13:44 ` Derrick Stolee
2023-04-23 1:18 ` Elijah Newren
2023-04-24 15:23 ` Derrick Stolee
2023-04-30 6:45 ` Elijah Newren
2023-09-03 15:47 ` Johannes Schindelin
2023-09-07 8:39 ` Christian Couder
2023-09-07 10:22 ` Johannes Schindelin [this message]
2023-04-17 15:45 ` Junio C Hamano
2023-04-18 5:58 ` Elijah Newren
2023-04-18 4:58 ` Elijah Newren
2023-04-15 18:30 ` Elijah Newren
2023-04-07 7:24 ` [PATCH 12/14] replay: introduce guess_new_base() Christian Couder
2023-04-07 7:24 ` [PATCH 13/14] replay: add different modes Christian Couder
2023-04-07 7:24 ` [PATCH 14/14] replay: stop assuming replayed branches do not diverge Christian Couder
2023-04-14 10:12 ` [PATCH 00/14] Introduce new `git replay` command Phillip Wood
2023-04-15 17:18 ` Elijah Newren
2023-04-14 17:39 ` Felipe Contreras
2023-04-15 6:44 ` Elijah Newren
2023-05-09 17:53 ` [PATCH v2 00/15] " Christian Couder
2023-05-09 17:53 ` [PATCH v2 01/15] t6429: remove switching aspects of fast-rebase Christian Couder
2023-05-09 17:53 ` [PATCH v2 02/15] replay: introduce new builtin Christian Couder
2023-05-09 17:53 ` [PATCH v2 03/15] replay: start using parse_options API Christian Couder
2023-05-09 17:53 ` [PATCH v2 04/15] replay: die() instead of failing assert() Christian Couder
2023-05-09 17:53 ` [PATCH v2 05/15] replay: introduce pick_regular_commit() Christian Couder
2023-05-09 17:53 ` [PATCH v2 06/15] replay: don't simplify history Christian Couder
2023-05-09 17:53 ` [PATCH v2 07/15] replay: add an important FIXME comment about gpg signing Christian Couder
2023-05-09 17:53 ` [PATCH v2 08/15] replay: remove progress and info output Christian Couder
2023-05-09 17:53 ` [PATCH v2 09/15] replay: remove HEAD related sanity check Christian Couder
2023-05-09 17:53 ` [PATCH v2 10/15] replay: make it a minimal server side command Christian Couder
2023-05-09 17:53 ` [PATCH v2 11/15] replay: use standard revision ranges Christian Couder
2023-05-09 17:53 ` [PATCH v2 12/15] replay: disallow revision specific options and pathspecs Christian Couder
2023-05-16 4:25 ` Elijah Newren
2023-05-09 17:53 ` [PATCH v2 13/15] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-05-09 17:53 ` [PATCH v2 14/15] replay: add --contained to rebase contained branches Christian Couder
2023-05-16 4:26 ` Elijah Newren
2023-05-09 17:53 ` [PATCH v2 15/15] replay: stop assuming replayed branches do not diverge Christian Couder
2023-05-16 4:26 ` Elijah Newren
2023-05-09 22:28 ` [PATCH v2 00/15] Introduce new `git replay` command Junio C Hamano
2023-05-10 7:33 ` Christian Couder
2023-05-16 4:42 ` Elijah Newren
2023-06-02 10:25 ` [PATCH v3 " Christian Couder
2023-06-02 10:25 ` [PATCH v3 01/15] t6429: remove switching aspects of fast-rebase Christian Couder
2023-06-02 10:25 ` [PATCH v3 02/15] replay: introduce new builtin Christian Couder
2023-06-02 10:25 ` [PATCH v3 03/15] replay: start using parse_options API Christian Couder
2023-06-02 10:25 ` [PATCH v3 04/15] replay: die() instead of failing assert() Christian Couder
2023-06-02 10:25 ` [PATCH v3 05/15] replay: introduce pick_regular_commit() Christian Couder
2023-06-02 10:25 ` [PATCH v3 06/15] replay: don't simplify history Christian Couder
2023-06-02 10:25 ` [PATCH v3 07/15] replay: add an important FIXME comment about gpg signing Christian Couder
2023-06-02 10:25 ` [PATCH v3 08/15] replay: remove progress and info output Christian Couder
2023-06-02 10:25 ` [PATCH v3 09/15] replay: remove HEAD related sanity check Christian Couder
2023-06-02 10:25 ` [PATCH v3 10/15] replay: make it a minimal server side command Christian Couder
2023-06-22 10:01 ` Toon Claes
2023-09-07 8:32 ` Christian Couder
2023-06-02 10:25 ` [PATCH v3 11/15] replay: use standard revision ranges Christian Couder
2023-06-22 10:03 ` Toon Claes
2023-09-07 8:32 ` Christian Couder
2023-09-07 21:02 ` Dragan Simic
2023-10-10 12:44 ` Christian Couder
2023-10-10 14:02 ` Dragan Simic
2023-06-02 10:25 ` [PATCH v3 12/15] replay: disallow revision specific options and pathspecs Christian Couder
2023-07-25 21:16 ` Junio C Hamano
2023-09-07 8:33 ` Christian Couder
2023-06-02 10:25 ` [PATCH v3 13/15] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-06-22 10:05 ` Toon Claes
2023-09-07 8:35 ` Christian Couder
2023-07-25 21:41 ` Junio C Hamano
2023-09-07 8:35 ` Christian Couder
2023-06-02 10:25 ` [PATCH v3 14/15] replay: add --contained to rebase contained branches Christian Couder
2023-06-22 10:10 ` Toon Claes
2023-09-07 8:37 ` Christian Couder
2023-06-02 10:25 ` [PATCH v3 15/15] replay: stop assuming replayed branches do not diverge Christian Couder
2023-06-03 1:42 ` [PATCH v3 00/15] Introduce new `git replay` command Junio C Hamano
2023-06-05 7:11 ` Christian Couder
2023-09-07 9:25 ` [PATCH v4 " Christian Couder
2023-09-07 9:25 ` [PATCH v4 01/15] t6429: remove switching aspects of fast-rebase Christian Couder
2023-09-07 9:25 ` [PATCH v4 02/15] replay: introduce new builtin Christian Couder
2023-09-07 10:23 ` Johannes Schindelin
2023-10-10 12:42 ` Christian Couder
2023-09-07 9:25 ` [PATCH v4 03/15] replay: start using parse_options API Christian Couder
2023-09-07 9:25 ` [PATCH v4 04/15] replay: die() instead of failing assert() Christian Couder
2023-09-07 9:25 ` [PATCH v4 05/15] replay: introduce pick_regular_commit() Christian Couder
2023-09-07 9:25 ` [PATCH v4 06/15] replay: don't simplify history Christian Couder
2023-09-07 10:23 ` Johannes Schindelin
2023-10-10 12:43 ` Christian Couder
2023-09-07 9:25 ` [PATCH v4 07/15] replay: add an important FIXME comment about gpg signing Christian Couder
2023-09-07 9:25 ` [PATCH v4 08/15] replay: remove progress and info output Christian Couder
2023-09-07 9:25 ` [PATCH v4 09/15] replay: remove HEAD related sanity check Christian Couder
2023-09-07 9:25 ` [PATCH v4 10/15] replay: make it a minimal server side command Christian Couder
2023-09-07 9:25 ` [PATCH v4 11/15] replay: use standard revision ranges Christian Couder
2023-09-07 10:24 ` Johannes Schindelin
2023-10-10 12:49 ` Christian Couder
2023-09-08 22:55 ` Linus Arver
2023-09-10 3:20 ` Linus Arver
2023-10-10 12:48 ` Christian Couder
2023-10-10 12:48 ` Christian Couder
2023-10-19 19:26 ` Linus Arver
2023-09-07 9:25 ` [PATCH v4 12/15] replay: disallow revision specific options and pathspecs Christian Couder
2023-09-07 10:24 ` Johannes Schindelin
2023-10-10 12:49 ` Christian Couder
2023-09-07 9:25 ` [PATCH v4 13/15] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-09-07 9:25 ` [PATCH v4 14/15] replay: add --contained to rebase contained branches Christian Couder
2023-09-07 9:25 ` [PATCH v4 15/15] replay: stop assuming replayed branches do not diverge Christian Couder
2023-09-07 10:25 ` [PATCH v4 00/15] Introduce new `git replay` command Johannes Schindelin
2023-10-10 12:50 ` Christian Couder
2023-10-10 12:38 ` [PATCH v5 00/14] " Christian Couder
2023-10-10 12:38 ` [PATCH v5 01/14] t6429: remove switching aspects of fast-rebase Christian Couder
2023-10-10 12:38 ` [PATCH v5 02/14] replay: introduce new builtin Christian Couder
2023-10-10 12:38 ` [PATCH v5 03/14] replay: start using parse_options API Christian Couder
2023-10-10 12:38 ` [PATCH v5 04/14] replay: die() instead of failing assert() Christian Couder
2023-10-10 12:38 ` [PATCH v5 05/14] replay: introduce pick_regular_commit() Christian Couder
2023-10-10 12:38 ` [PATCH v5 06/14] replay: change rev walking options Christian Couder
2023-10-10 12:38 ` [PATCH v5 07/14] replay: add an important FIXME comment about gpg signing Christian Couder
2023-10-10 12:38 ` [PATCH v5 08/14] replay: remove progress and info output Christian Couder
2023-10-10 12:38 ` [PATCH v5 09/14] replay: remove HEAD related sanity check Christian Couder
2023-10-10 12:38 ` [PATCH v5 10/14] replay: make it a minimal server side command Christian Couder
2023-10-10 12:38 ` [PATCH v5 11/14] replay: use standard revision ranges Christian Couder
2023-10-19 19:49 ` Linus Arver
2023-10-10 12:38 ` [PATCH v5 12/14] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-10-10 12:38 ` [PATCH v5 13/14] replay: add --contained to rebase contained branches Christian Couder
2023-10-10 12:38 ` [PATCH v5 14/14] replay: stop assuming replayed branches do not diverge Christian Couder
2023-10-26 13:44 ` [PATCH v5 00/14] Introduce new `git replay` command Johannes Schindelin
2023-10-29 6:01 ` Elijah Newren
2023-11-02 14:59 ` Christian Couder
2023-11-08 12:25 ` Johannes Schindelin
2023-11-02 15:06 ` Christian Couder
2023-11-08 12:25 ` Johannes Schindelin
2023-10-29 6:00 ` Elijah Newren
2023-10-29 14:14 ` Johannes Schindelin
2023-10-30 17:18 ` Elijah Newren
2023-11-02 14:44 ` Christian Couder
2023-11-02 14:48 ` Christian Couder
2023-11-02 13:51 ` [PATCH v6 " Christian Couder
2023-11-02 13:51 ` [PATCH v6 01/14] t6429: remove switching aspects of fast-rebase Christian Couder
2023-11-02 13:51 ` [PATCH v6 02/14] replay: introduce new builtin Christian Couder
2023-11-02 13:51 ` [PATCH v6 03/14] replay: start using parse_options API Christian Couder
2023-11-02 13:51 ` [PATCH v6 04/14] replay: die() instead of failing assert() Christian Couder
2023-11-02 13:51 ` [PATCH v6 05/14] replay: introduce pick_regular_commit() Christian Couder
2023-11-02 13:51 ` [PATCH v6 06/14] replay: change rev walking options Christian Couder
2023-11-02 13:51 ` [PATCH v6 07/14] replay: add an important FIXME comment about gpg signing Christian Couder
2023-11-02 13:51 ` [PATCH v6 08/14] replay: remove progress and info output Christian Couder
2023-11-02 13:51 ` [PATCH v6 09/14] replay: remove HEAD related sanity check Christian Couder
2023-11-02 13:51 ` [PATCH v6 10/14] replay: make it a minimal server side command Christian Couder
2023-11-02 13:51 ` [PATCH v6 11/14] replay: use standard revision ranges Christian Couder
2023-11-02 13:51 ` [PATCH v6 12/14] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-11-02 13:51 ` [PATCH v6 13/14] replay: add --contained to rebase contained branches Christian Couder
2023-11-02 13:51 ` [PATCH v6 14/14] replay: stop assuming replayed branches do not diverge Christian Couder
2023-11-07 2:43 ` [PATCH v6 00/14] Introduce new `git replay` command Elijah Newren
2023-11-07 9:43 ` Christian Couder
2023-11-15 14:51 ` Christian Couder
2023-11-08 12:19 ` Johannes Schindelin
2023-11-08 12:47 ` Johannes Schindelin
2023-11-15 14:46 ` Christian Couder
2023-11-16 8:45 ` Johannes Schindelin
2023-11-16 8:52 ` Christian Couder
2023-11-15 14:33 ` [PATCH v7 " Christian Couder
2023-11-15 14:33 ` [PATCH v7 01/14] t6429: remove switching aspects of fast-rebase Christian Couder
2023-11-15 14:33 ` [PATCH v7 02/14] replay: introduce new builtin Christian Couder
2023-11-15 14:33 ` [PATCH v7 03/14] replay: start using parse_options API Christian Couder
2023-11-15 14:33 ` [PATCH v7 04/14] replay: die() instead of failing assert() Christian Couder
2023-11-15 14:33 ` [PATCH v7 05/14] replay: introduce pick_regular_commit() Christian Couder
2023-11-15 14:33 ` [PATCH v7 06/14] replay: change rev walking options Christian Couder
2023-11-15 14:33 ` [PATCH v7 07/14] replay: add an important FIXME comment about gpg signing Christian Couder
2023-11-15 14:33 ` [PATCH v7 08/14] replay: remove progress and info output Christian Couder
2023-11-15 14:33 ` [PATCH v7 09/14] replay: remove HEAD related sanity check Christian Couder
2023-11-15 14:33 ` [PATCH v7 10/14] replay: make it a minimal server side command Christian Couder
2023-11-15 14:33 ` [PATCH v7 11/14] replay: use standard revision ranges Christian Couder
2023-11-15 14:33 ` [PATCH v7 12/14] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-11-15 14:33 ` [PATCH v7 13/14] replay: add --contained to rebase contained branches Christian Couder
2023-11-15 14:33 ` [PATCH v7 14/14] replay: stop assuming replayed branches do not diverge Christian Couder
2023-11-16 8:53 ` [PATCH v7 00/14] Introduce new `git replay` command Johannes Schindelin
2023-11-23 19:32 ` Elijah Newren
2023-11-24 0:28 ` Junio C Hamano
2023-11-24 11:10 ` [PATCH v8 " Christian Couder
2023-11-24 11:10 ` [PATCH v8 01/14] t6429: remove switching aspects of fast-rebase Christian Couder
2023-11-24 11:10 ` [PATCH v8 02/14] replay: introduce new builtin Christian Couder
2023-11-24 11:10 ` [PATCH v8 03/14] replay: start using parse_options API Christian Couder
2023-11-24 11:10 ` [PATCH v8 04/14] replay: die() instead of failing assert() Christian Couder
2023-11-24 11:10 ` [PATCH v8 05/14] replay: introduce pick_regular_commit() Christian Couder
2023-11-24 11:10 ` [PATCH v8 06/14] replay: change rev walking options Christian Couder
2023-11-24 11:10 ` [PATCH v8 07/14] replay: add an important FIXME comment about gpg signing Christian Couder
2023-11-24 11:10 ` [PATCH v8 08/14] replay: remove progress and info output Christian Couder
2023-11-24 11:10 ` [PATCH v8 09/14] replay: remove HEAD related sanity check Christian Couder
2023-11-24 11:10 ` [PATCH v8 10/14] replay: make it a minimal server side command Christian Couder
2023-11-24 11:10 ` [PATCH v8 11/14] replay: use standard revision ranges Christian Couder
2023-11-24 11:10 ` [PATCH v8 12/14] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-11-24 11:10 ` [PATCH v8 13/14] replay: add --contained to rebase contained branches Christian Couder
2023-11-24 11:10 ` [PATCH v8 14/14] replay: stop assuming replayed branches do not diverge Christian Couder
2023-11-25 0:02 ` [PATCH v8 00/14] Introduce new `git replay` command Johannes Schindelin
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=4a648214-2627-c9c0-0be2-5a694e8c3b3c@gmx.de \
--to=johannes.schindelin@gmx.de \
--cc=chriscool@tuxfamily.org \
--cc=christian.couder@gmail.com \
--cc=derrickstolee@github.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johncai86@gmail.com \
--cc=newren@gmail.com \
--cc=ps@pks.im \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).