Re: [PATCH 1/7] rebase --apply: remove duplicated code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 22, 2022 at 6:34 AM Phillip Wood via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
>
> 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.

> 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.

> Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
> ---
>  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.

>         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!



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux