Re: [PATCH 0/8] sequencer refactoring

From: Phillip Wood <phillip.wood123@gmail.com>
Date: May 17, 2023, 13:10 UTC
Message-ID: <a6e31eb9-81e7-4d7f-28cc-73b5e46525a4@gmail.com>
In-Reply-To: <20230323162235.995574-1-oswald.buddenhagen@gmx.de>

Hi Oswald

On 23/03/2023 16:22, Oswald Buddenhagen wrote:

> This is a preparatory series for the separately posted 'rebase --rewind' patch,
> but I think it has value in itself.

When you re-roll this I think it would be worth splitting it into two separate series.

Patches 1, 2, 4 & 6 are simple clean ups which don't need much work beyond making sure that (a) the commit messages have a good explanation of the reason for the change (try "git log --author "Jeff King" for examples of good commit messages) and (b) the code follows our coding guidelines (mostly no '//' comments if I remember correctly).

Patches 5 & 8 address real problems but are more involved and it will take more time to consider the UI changes and get them right.

I'd rather we dropped patches 3 & 7.

Best Wishes

Phillip

Show 21 quoted lines

> 
> Oswald Buddenhagen (8):
>    rebase: simplify code related to imply_merge()
>    rebase: move parse_opt_keep_empty() down
>    sequencer: pass around rebase action explicitly
>    sequencer: create enum for edit_todo_list() return value
>    rebase: preserve interactive todo file on checkout failure
>    sequencer: simplify allocation of result array in
>      todo_list_rearrange_squash()
>    sequencer: pass `onto` to complete_action() as object-id
>    rebase: improve resumption from incorrect initial todo list
> 
>   builtin/rebase.c              |  63 +++++++--------
>   builtin/revert.c              |   3 +-
>   rebase-interactive.c          |  36 ++++-----
>   rebase-interactive.h          |  27 ++++++-
>   sequencer.c                   | 139 +++++++++++++++++++---------------
>   sequencer.h                   |  15 ++--
>   t/t3404-rebase-interactive.sh |  34 ++++++++-
>   7 files changed, 196 insertions(+), 121 deletions(-)
>