Re: [PATCH 1/1] replay: add --revert option to reverse commit changes

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Siddharth

On 25/11/2025 17:00, Siddharth Asthana wrote:
> 
> diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc
> index dcb26e8a8e..ad7dc08622 100644
> --- a/Documentation/git-replay.adoc
> +++ b/Documentation/git-replay.adoc
> @@ -54,6 +54,18 @@ which uses the target only as a starting point without updating it.
> [...]
> +To revert a range of commits:
> +
> +------------
> +$ git replay --revert --onto main feature~3..feature
> +------------
> +
> +This creates new commits on top of 'main' that reverse the changes introduced
> +by the last three commits on 'feature'. The 'feature' branch is updated to
> +point at the last of these revert commits. The 'main' branch is not updated
> +in this case.

I'm struggling to understand when I'd want to do this. Why would I want 
to update 'feature' to point to the reverted version of its last tree 
commits rebased onto 'main'? In order to understand I ran the first 
tests case which does

	git replay --onto topic1 --revert topic1..topic2

after fixing it by adding --ref-action=print the resulting commit log 
looks like

commit d337fab78e90008835f74e890039b464a0308cbe
Author: author@name <bogus@email@address>
Date:   Thu Apr 7 15:30:13 2005 -0700

     Revert "E
     "

     This reverts commit bceb3acd81ddd36ba0da391fffa48949a1337276.

commit 47f0cc1c1f1911c0047a4d79d79f7c19c6c7151a
Author: author@name <bogus@email@address>
Date:   Thu Apr 7 15:30:13 2005 -0700

     Revert "D
     "

     This reverts commit d953cf2dcc1da8b51934e43fd83dac72d0e267c7.


The commits are empty because the original they are reverting each 
create a new file which is then present in the base revision but not in 
either of the merge heads when we revert. This suggests to me that it is 
not a very realistic test and I'm still scratching my head to see where 
"git replay --onto <commit> --revert" is useful.

If '--revert' does not make sense with '--onto' then perhaps it should 
be a new mode that takes a ref and acts like '--advance' but reverts the 
commits rather than cherry-picking them. When reverting a range of 
commits it would reduce the likelihood of conflicts to revert then in 
reverse order so we should either recommend passing '--reverse' or make 
that the default when '--revert' is given.

As you can see in the log output above the new function to format the 
revert subject lines is buggy. If you had used test_commit_message() to 
check the commit message, rather than just grepping for ^Revert the 
tests would have picked that up.

Thanks

Phillip