Re: [PATCH v3 6/8] rebase: factor out branch_base calculation

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Oct 17 2022, Phillip Wood wrote:

> On 13/10/2022 20:21, Ævar Arnfjörð Bjarmason wrote:
>>> +static void fill_branch_base(struct rebase_options *options,
>>> +			    struct object_id *branch_base)
>>> +{
>>> +	struct commit_list *merge_bases = NULL;
>>> +
>>> +	merge_bases = get_merge_bases(options->onto, options->orig_head);
>>> +	if (!merge_bases || merge_bases->next)
>>> +		oidcpy(branch_base, null_oid());
>>> +	else
>>> +		oidcpy(branch_base, &merge_bases->item->object.oid);
>>> +
>>> +	free_commit_list(merge_bases);
>>> +}
>> I wondered if this could be a bit shorter/less wrap-y
>
> Where's the wrapping?

Sorry about being unclear, I meant (but completely failed to get across)
that you seemed to be pre-declaring the "merge_bases" to avoid wrapping
the "get_merge_bases()" line.

But reading it again maybe it was just copied as-is from the
pre-image. In any case as we're moving this to a new function maybe a
fix-up to make it:

	struct commit_list *merge_bases = get_merge_bases(options->onto,
							  options->orig_head);

would be marginally easier to read, as we never use that NULL-init
(which again, is also an issue in the pre-image).

Anyway, if you want to keep this all as-is that's fine with me.

>> with shorter
>> variable names, anyway, I see it's code copied from above, so nevermind
>> in advance... :)
>
> As it is copied it is easier to review leaving it as is I think.

*nod*