Re: [PATCH v4 07/12] rebase: add --update-refs option
- From
Elijah Newren <newren@gmail.com>
- Date
- Jul 16, 2022, 19:30 UTC
- Message-ID
- <CABPp-BENsp3uc7J_xS0gr_yjWQ25NVuJbdHzCMEcaaGYvLEiKA@mail.gmail.com>
- In-Reply-To
- <3ec2cc922f971af4e4a558188cf139cc0c0150d6.1657631226.git.gitgitgadget@gmail.com>
On Tue, Jul 12, 2022 at 6:07 AM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:
> > From: Derrick Stolee <derrickstolee@github.com> >
[...]
Show 7 quoted lines
> > +--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.
I think the second sentence here should be split. In particular, I don't think I understand the second half of the second sentence. Do you intend to say here that branches pointing to a `squash!` or `fixup!` will instead update the first `pick` in the ancestry of such a commit, rather than that such branches are entirely excluded from any updates? That's what I observed in my testing of your v3, at least, and that's the behavior I'd expect this feature to implement, but this documentation doesn't match.
[...]
Show 7 quoted lines
> @@ -5660,6 +5764,9 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla > item->arg_len = item->arg_offset = item->flags = item->offset_in_buf = 0; > } > > + if (update_refs && todo_list_add_update_ref_commands(todo_list)) > + return -1; > +
As a tangent, I find it interesting that you add the update-ref commands as a post-processing step rather than as a part of sequencer_make_script(). I don't think you need to change anything, but I am curious due to my git-replay work if you find it advantageous to do it this way.