Re: [PATCH 3/4] rebase: add --update-refs option
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Jun 7, 2022, 10:25 UTC
- Message-ID
- <c9a5369a-8c87-0ee8-bed7-16b2cd58717b@gmail.com>
- In-Reply-To
- <23fa663886405596370e5fc95f679ef299f0725c.1654263472.git.gitgitgadget@gmail.com>
Hi Stolee
Just a couple of minor comments.
On 03/06/2022 14:37, Derrick Stolee via GitGitGadget wrote:
Show 19 quoted lines
> From: Derrick Stolee <derrickstolee@github.com> > [...] > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > index 262fb01aec0..866554fc978 100644 > --- a/Documentation/git-rebase.txt > +++ b/Documentation/git-rebase.txt > @@ -609,6 +609,13 @@ provided. Otherwise an explicit `--no-reschedule-failed-exec` at the > start would be overridden by the presence of > `rebase.rescheduleFailedExec=true` configuration. > > +--update-refs:: > +--no-update-refs:: > + Automatically force-update any branches that point to commits that > + are being rebased. Any branches that are checked out in a worktree > + or point to a `squash! ...` or `fixup! ...` commit are not updated > + in this way. > + > INCOMPATIBLE OPTIONS > --------------------
We should add --update-refs to the list of options that are incompatible with --apply.
Show 19 quoted lines
> diff --git a/sequencer.c b/sequencer.c
> index 8c3ed3532ac..d6151af9849 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -35,6 +35,8 @@
> #include "commit-reach.h"
> #include "rebase-interactive.h"
> #include "reset.h"
> +#include "branch.h"
> +#include "log-tree.h"
>
> #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
>
> @@ -5603,10 +5605,104 @@ static int skip_unnecessary_picks(struct repository *r,
> return 0;
> }
>
> +struct todo_add_branch_context {
> + struct todo_list new_list;Rather than using a struct todo_list I think it would be simpler overall to add struct todo_list_item *item; size_t nr, alloc;
instead, as I found it confusing that we were (correctly) using the strbuf of the old list when adding the update-ref line to the new list.
Show 16 quoted lines
> + struct strbuf *buf;
> + struct commit *commit;
> +};
> [...]
> + for (i = 0; i < todo_list->nr; ) {
> + struct todo_item *item = &todo_list->items[i];
> +
> + do {
> + /* insert ith item into new list */
> + ALLOC_GROW(ctx.new_list.items,
> + ctx.new_list.nr + 1,
> + ctx.new_list.alloc);
> +
> + memcpy(&ctx.new_list.items[ctx.new_list.nr++],
> + &todo_list->items[i],
> + sizeof(struct todo_item));May be ctx.new_list.items[ctx.new_list.nr++] = todo_list->items[i++]; would be clearer
Best Wishes
Phillip