From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 26 Jun 2019 16:27:06 GMT
Subject: Re: [PATCH 2/6] tree-walk.c: remove the_repo from fill_tree_descriptor()
Message-ID: <xmqqblyke2hx.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <a2d91c63-86ca-90e7-0087-09df269d3865@gmail.com>

Derrick Stolee <stolee@gmail.com> writes:

>> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
>> index 34ca0258b1..97b54caeb9 100644
>> --- a/builtin/merge-tree.c
>> +++ b/builtin/merge-tree.c
>> @@ -205,6 +205,7 @@ static void resolve(const struct traverse_info *info, struct name_entry *ours, s
>>  static void unresolved_directory(const struct traverse_info *info,
>>  				 struct name_entry n[3])
>>  {
>> +	struct repository *r = the_repository;
>
> I like this trick to make the change below minimal:
>> +	buf0 = fill_tree_descriptor(r, t + 0, ENTRY_OID(n + 0));
>> +	buf1 = fill_tree_descriptor(r, t + 1, ENTRY_OID(n + 1));
>> +	buf2 = fill_tree_descriptor(r, t + 2, ENTRY_OID(n + 2));
>
> I wonder if _every_ conversion should include this trick,
> so when we move to change that method we simply move the definition
> from the method block to the prototype. (No need to adjust what you've
> done already, just an idea for future conversions.)

Hmm, interesting.  So those callers in builtin/rebase.c::reset_head()
and other places that adds the_repository as the new first parameter
can take a local variable "r" (or perhaps a bit more descriptive,
e.g. "repo") that is initialized to "the_repository" (and never
reassigned at least at this step) in this same patch?