Re: [PATCH 2/6] tree-walk.c: remove the_repo from fill_tree_descriptor()

[Derrick Stolee <stolee@gmail.com>]

On 6/26/2019 12:27 PM, Junio C Hamano wrote:
> 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?

Yes, that is what I was thinking. It means that we can stop munging
the function calls for the method that is converted. When the caller
is updated, the call site already uses "r" but we change how "r" is
initialized (as a parameter instead of a local variable).

Thanks,
-Stolee