diff mbox series

[v2,05/14] pull: trivial cleanup

Message ID 20201204061623.1170745-6-felipe.contreras@gmail.com (mailing list archive)
State New, archived
Headers show
Series pull: default warning improvements | expand

Commit Message

Felipe Contreras Dec. 4, 2020, 6:16 a.m. UTC
There's no need to store ran_ff. Now it's obvious from the conditionals.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/pull.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Elijah Newren Dec. 4, 2020, 11:09 p.m. UTC | #1
On Thu, Dec 3, 2020 at 10:16 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> There's no need to store ran_ff. Now it's obvious from the conditionals.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  builtin/pull.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index c38548dab8..8daba7539c 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -1024,19 +1024,18 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
>
>         if (opt_rebase) {
>                 int ret = 0;
> -               int ran_ff = 0;
>                 if ((recurse_submodules == RECURSE_SUBMODULES_ON ||
>                      recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND) &&
>                     submodule_touches_in_range(the_repository, &rebase_fork_point, &curr_head))
>                         die(_("cannot rebase with locally recorded submodule modifications"));
> +
spurious change?

(It does make it look nicer, though.)

>                 if (get_can_ff(&orig_head, &merge_heads.oid[0])) {
>                         /* we can fast-forward this without invoking rebase */
>                         opt_ff = "--ff-only";
> -                       ran_ff = 1;
>                         ret = run_merge();
> -               }
> -               if (!ran_ff)
> +               } else {
>                         ret = run_rebase(&curr_head, merge_heads.oid, &rebase_fork_point);
> +               }
>
>                 if (!ret && (recurse_submodules == RECURSE_SUBMODULES_ON ||
>                              recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))
> --
> 2.29.2

Yeah, reads better to me after this cleanup.
Felipe Contreras Dec. 5, 2020, 12:48 a.m. UTC | #2
On Fri, Dec 4, 2020 at 5:09 PM Elijah Newren <newren@gmail.com> wrote:
>
> On Thu, Dec 3, 2020 at 10:16 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > There's no need to store ran_ff. Now it's obvious from the conditionals.
> >
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > ---
> >  builtin/pull.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/builtin/pull.c b/builtin/pull.c
> > index c38548dab8..8daba7539c 100644
> > --- a/builtin/pull.c
> > +++ b/builtin/pull.c
> > @@ -1024,19 +1024,18 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
> >
> >         if (opt_rebase) {
> >                 int ret = 0;
> > -               int ran_ff = 0;
> >                 if ((recurse_submodules == RECURSE_SUBMODULES_ON ||
> >                      recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND) &&
> >                     submodule_touches_in_range(the_repository, &rebase_fork_point, &curr_head))
> >                         die(_("cannot rebase with locally recorded submodule modifications"));
> > +
> spurious change?
>
> (It does make it look nicer, though.)

Nope. Was intentional.

> >                 if (get_can_ff(&orig_head, &merge_heads.oid[0])) {
> >                         /* we can fast-forward this without invoking rebase */
> >                         opt_ff = "--ff-only";
> > -                       ran_ff = 1;
> >                         ret = run_merge();
> > -               }
> > -               if (!ran_ff)
> > +               } else {
> >                         ret = run_rebase(&curr_head, merge_heads.oid, &rebase_fork_point);
> > +               }
> >
> >                 if (!ret && (recurse_submodules == RECURSE_SUBMODULES_ON ||
> >                              recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))
> > --
> > 2.29.2
>
> Yeah, reads better to me after this cleanup.
diff mbox series

Patch

diff --git a/builtin/pull.c b/builtin/pull.c
index c38548dab8..8daba7539c 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1024,19 +1024,18 @@  int cmd_pull(int argc, const char **argv, const char *prefix)
 
 	if (opt_rebase) {
 		int ret = 0;
-		int ran_ff = 0;
 		if ((recurse_submodules == RECURSE_SUBMODULES_ON ||
 		     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND) &&
 		    submodule_touches_in_range(the_repository, &rebase_fork_point, &curr_head))
 			die(_("cannot rebase with locally recorded submodule modifications"));
+
 		if (get_can_ff(&orig_head, &merge_heads.oid[0])) {
 			/* we can fast-forward this without invoking rebase */
 			opt_ff = "--ff-only";
-			ran_ff = 1;
 			ret = run_merge();
-		}
-		if (!ran_ff)
+		} else {
 			ret = run_rebase(&curr_head, merge_heads.oid, &rebase_fork_point);
+		}
 
 		if (!ret && (recurse_submodules == RECURSE_SUBMODULES_ON ||
 			     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))