Re: [PATCH 2/6] tree-walk.c: remove the_repo from fill_tree_descriptor()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 26, 2019, 16:27 UTC
- 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:
Show 19 quoted lines
>> 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?