Hi, On Thu, Oct 26, 2023 at 6:44 AM Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote: > > Hi Christian, > > On Tue, 10 Oct 2023, Christian Couder wrote: > [...] > > + /* requirements/overrides for revs */ > > -+ revs.reverse = 1; > > ++ revs.reverse = !revs.reverse; > > + revs.sort_order = REV_SORT_IN_GRAPH_ORDER; > > + revs.topo_order = 1; > > + revs.simplify_history = 0; > > This still overrides a couple of command-line options, _silently_. I would > prefer those three assignments to be moved just before the > `setup_revisions()` call. > > Letting users override these settings may not make much sense, but it > makes even less sense to pretend to let them override the settings and > then just ignore them without warning. (See also > https://en.wikipedia.org/wiki/Principle_of_least_astonishment.) > > Moving these three assignments before the `setup_revisions()` call would > neatly remedy that. I agree that warnings or error messages would be better. But if we're talking about something short of that, I'd actually argue the opposite of what you do here. I intentionally moved these assignments after setup_revisions(), and in my mind, the purpose in doing so was to satisfy the Principle of Least Astonishment. My experience with git-fast-export, where some settings are made before calling setup_revisions() and then can be overridden, and then do completely hideous things, was much worse to me than just admitting the flags are bad given the various assumptions the tool makes. I have some patches sitting around to fix fast-export that I never got around to upstreaming, but when it came time to implement git-replay, I made sure to fix what I viewed as the bigger problem. [...] > > @@ Documentation/git-replay.txt (new) > > + > > +NAME > > +---- > > -+git-replay - Replay commits on a different base, without touching working tree > > ++git-replay - Replay commits on a new base, works on bare repos too > > + > > + > > +SYNOPSIS > > As mentioned in > https://lore.kernel.org/git/03460733-0219-c648-5757-db1958f8042e@xxxxxx/, > I would like the `EXPERIMENTAL` label to be shown prominently here. > Probably not only the `SYNOPSIS` as I had originally suggested but also in > the `NAME`. > > Otherwise we may end up with the same situation as with the (from my > perspective, failed) `git switch`/`git restore` experiment, where we > wanted to explore a better user experience than the overloaded `git > checkout` command, only to now be stuck with having to maintain > backward-compatibility for `git switch`/`git restore` command-line options > that were not meant to be set in stone but to be iterated on, instead. A > real-life demonstration of [Hyrum's Law](hyrumslaw.com/), if you like. Or, > from a different angle, we re-enacted https://xkcd.com/927/ in a way. > > I'd like to suggest to learn from history and avoid this by tacking on a > warning label right at the top of the documentation. We may eventually > manage to iterate `git replay` to a point where it is totally capable to > supersede `git rebase`, by doing everything the latter does, except > better, who knows? But we _do_ need the liberty to make sweeping changes > to this new builtin if we want to have a prayer of doing that. And I fear > that not even mentioning the EXPERIMENTAL nature right at the top of the > manual page would just render us into that undesirable corner. I fully support this. Absolutely, 100%. Thanks both, Elijah
