Hi Jonathan

Thanks for taking a look at this series

On 24/08/2022 23:02, Jonathan Tan wrote:

"Phillip Wood via GitGitGadget" [off-list ref] writes:

quoted

@@ -1668,7 +1678,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
  			die(_("Does not point to a valid commit '%s'"),
  				options.onto_name);
  	}
-
+	if (keep_base) {
+		oidcpy(&merge_base, &options.onto->object.oid);
+	} else {
+		fill_merge_base(&options, &merge_base);
+	}
  	if (options.fork_point > 0)
  		options.restrict_revision =
  			get_fork_point(options.upstream_name, options.orig_head);

This patch makes sense, except for this part: why is fill_merge_base()
only called for non-keep_base, but it seemed to be unconditionally
called before? For what it's worth, all tests pass even with this diff:

It's an optimization, we have already calculated the merge base above in 
the "onto" calculation when --keep-base is given. This is what I meant 
by "avoid calculating the merge-base twice when --keep-base is given" in 
the commit message, maybe I should  try and come up with some clearer 
wording.

Best Wishes

Phillip

   -       if (keep_base) {
   -               oidcpy(&merge_base, &options.onto->object.oid);
   -       } else {
   -               fill_merge_base(&options, &merge_base);
   -       }
   +       fill_merge_base(&options, &merge_base);