Hi Jonathan
Thanks for taking a look at this series
On 24/08/2022 23:02, Jonathan Tan wrote:
"Phillip Wood via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
@@ -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);
