Re: [PATCH 1/7] rebase --apply: remove duplicated code
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Apr 7, 2022, 13:35 UTC
- Message-ID
- <CAP8UFD25KjNYuMoVb72N2kGmo3yJ0qGgfJn78axc562Z+YhAGQ@mail.gmail.com>
- In-Reply-To
- <243e7b0c372c15cc907c838ee74794433aa79858.1645441854.git.gitgitgadget@gmail.com>
On Tue, Feb 22, 2022 at 6:34 AM Phillip Wood via GitGitGadget <gitgitgadget@gmail.com> wrote:
Show 6 quoted lines
> > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > When we are reattaching HEAD after a fast-forward we can use > move_to_original_branch() rather than open coding a copy of that > code.
Nit: maybe "rebase --apply: reuse move_to_original_branch()" would be a better subject for this patch, as it's more specific.
Show 6 quoted lines
> The old code appears to handle the case where the rebase is > started from a detached HEAD but in fact in that case there is nothing > to do as we have already updated HEAD. > > Note that the removal of "strbuf_release(&msg)" is safe as there is an > identical call just above this hunk.
It looks like the removed call is using strbuf_reset(), not strbuf_release().
Actually a call to strbuf_release(&msg) is also removed inside the `if (oideq(&merge_base, &options.orig_head)) { ... }` clause, but it doesn't look like you are talking about this one.
Show 14 quoted lines
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- > builtin/rebase.c | 11 +---------- > 1 file changed, 1 insertion(+), 10 deletions(-) > > diff --git a/builtin/rebase.c b/builtin/rebase.c > index e942c300f8c..4832f16e675 100644 > --- a/builtin/rebase.c > +++ b/builtin/rebase.c > @@ -1782,19 +1782,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > * If the onto is a proper descendant of the tip of the branch, then > * we just fast-forwarded. > */ > - strbuf_reset(&msg);
Yeah, it looks like `msg` isn't used in the function anymore so this call can be removed.
Show 16 quoted lines
> if (oideq(&merge_base, &options.orig_head)) {
> printf(_("Fast-forwarded %s to %s.\n"),
> branch_name, options.onto_name);
> - strbuf_addf(&msg, "rebase finished: %s onto %s",
> - options.head_name ? options.head_name : "detached HEAD",
> - oid_to_hex(&options.onto->object.oid));
> - memset(&ropts, 0, sizeof(ropts));
> - ropts.branch = options.head_name;
> - ropts.flags = RESET_HEAD_REFS_ONLY;
> - ropts.head_msg = msg.buf;
> - reset_head(the_repository, &ropts);
> - strbuf_release(&msg);
> + move_to_original_branch(&options);
> ret = finish_rebase(&options);
> goto cleanup;
> }Nice cleanup!