Re: [PATCH 00/12] Remove more index compatibility macros

> On Sun, Jan 3, 2021 at 8:01 PM Derrick Stolee <stolee@gmail.com> wrote:
>> On 1/2/2021 1:12 AM, Eric Sunshine wrote:
>>> This should allow you to get rid of the globals introduced by patch
>>> [4/12] (assuming passing the index and repo arguments around
>>> everywhere doesn't get overly hairy).
>>
>> Perhaps now that I've removed the compatibility macros, it would be
>> easier to insert the method parameters since most of the lines that
>> need to change would be method prototypes and the calls to those methods
>> (plus the callback function details).
>>
>> Is that a valuable effort? I could give it a try, but I want to be sure
>> that adjusting all of those helper methods in the builtin would indeed
>> have valuable improvements over the static globals used here.
> 
> My impression was that the goal of the earlier work was to pass the
> index and repository to each function specifically to avoid tying the
> function to a particular index or repository. This helps in cases in
> which client code needs to operate on a different index or repository
> (for instance, a submodule). Generally speaking, making the index and
> repository file-static rather than global does not help reach that
> goal since functions are still tied to state which is not local to the
> function itself.
> 
> Would the extra effort be valuable in this particular case? I'm not
> familiar with this code, but given that `update-index` is a builtin,
> such effort may not be too meaningful. If, however, any of the code
> from `buildin/update-index.c` ever gets "libified" and moved into the
> core library, then that would be a good time to update the functions
> to take those values as arguments rather than relying on file-static
> or globals. But that's not something that this series necessarily
> needs to do; the task can wait until the code needs to be shared by
> other modules, I would think.