mbox series

[0/4] rebase: cleanup merge strategy option handling

Message ID cover.1678893298.git.phillip.wood@dunelm.org.uk (mailing list archive)
Headers show
Series rebase: cleanup merge strategy option handling | expand

Message

Phillip Wood March 15, 2023, 3:14 p.m. UTC
From: Phillip Wood <phillip.wood@dunelm.org.uk>

Cleanup the handling of --strategy-option now that we no longer need
to support "--preserve-merges" and properly quote the argument when
saving it to disc.

These patches are based on a merge of 'master' and
'ab/fix-strategy-opts-parsing'

Published-As: https://github.com/phillipwood/git/releases/tag/sequencer-merge-strategy-options%2Fv1
View-Changes-At: https://github.com/phillipwood/git/compare/c2e329a52...3e02eeff7
Fetch-It-Via: git fetch https://github.com/phillipwood/git sequencer-merge-strategy-options/v1

Phillip Wood (4):
  rebase: stop reading and writing unnecessary strategy state
  rebase -m: cleanup --strategy-option handling
  rebase -m: fix serialization of strategy options
  rebase: remove a couple of redundant strategy tests

 builtin/rebase.c               | 60 +++++++++-----------------------
 sequencer.c                    | 26 ++++++++++----
 sequencer.h                    |  1 -
 t/t3402-rebase-merge.sh        | 21 ------------
 t/t3418-rebase-continue.sh     | 62 +++++++++++-----------------------
 t/t3436-rebase-more-options.sh | 18 ----------
 6 files changed, 56 insertions(+), 132 deletions(-)

Comments

Junio C Hamano March 15, 2023, 4:03 p.m. UTC | #1
Phillip Wood <phillip.wood123@gmail.com> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Cleanup the handling of --strategy-option now that we no longer need
> to support "--preserve-merges" and properly quote the argument when
> saving it to disc.

Nice.

>
> These patches are based on a merge of 'master' and
> 'ab/fix-strategy-opts-parsing'
>
> Published-As: https://github.com/phillipwood/git/releases/tag/sequencer-merge-strategy-options%2Fv1
> View-Changes-At: https://github.com/phillipwood/git/compare/c2e329a52...3e02eeff7
> Fetch-It-Via: git fetch https://github.com/phillipwood/git sequencer-merge-strategy-options/v1
>
> Phillip Wood (4):
>   rebase: stop reading and writing unnecessary strategy state
>   rebase -m: cleanup --strategy-option handling
>   rebase -m: fix serialization of strategy options
>   rebase: remove a couple of redundant strategy tests
>
>  builtin/rebase.c               | 60 +++++++++-----------------------
>  sequencer.c                    | 26 ++++++++++----
>  sequencer.h                    |  1 -
>  t/t3402-rebase-merge.sh        | 21 ------------
>  t/t3418-rebase-continue.sh     | 62 +++++++++++-----------------------
>  t/t3436-rebase-more-options.sh | 18 ----------
>  6 files changed, 56 insertions(+), 132 deletions(-)
Felipe Contreras March 15, 2023, 4:50 p.m. UTC | #2
On Wed, Mar 15, 2023 at 9:39 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Cleanup the handling of --strategy-option now that we no longer need
> to support "--preserve-merges" and properly quote the argument when
> saving it to disc.
>
> These patches are based on a merge of 'master' and
> 'ab/fix-strategy-opts-parsing'
>
> Published-As: https://github.com/phillipwood/git/releases/tag/sequencer-merge-strategy-options%2Fv1
> View-Changes-At: https://github.com/phillipwood/git/compare/c2e329a52...3e02eeff7
> Fetch-It-Via: git fetch https://github.com/phillipwood/git sequencer-merge-strategy-options/v1

FWIW I reviewed the changes and they all look good to me.
Elijah Newren April 1, 2023, 7:32 p.m. UTC | #3
On Wed, Mar 15, 2023 at 8:39 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Cleanup the handling of --strategy-option now that we no longer need
> to support "--preserve-merges" and properly quote the argument when
> saving it to disc.

Nice!  Thanks for taking the time to do these kinds of cleanups.
Overall the code looks pretty good, but I left a few
comments/questions on patches 3 & 4.