Re: [PATCH v3 2/4] refs: introduce get_worktree_ref_store()

[Michael Haggerty <mhagger@alum.mit.edu>]

On 03/20/2017 01:01 PM, Duy Nguyen wrote:
> On Mon, Mar 20, 2017 at 1:59 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> I guess I can hold my nose and accept storing worktree and submodule
>> `ref_store`s together in a single hashmap
> 
> Release your nose, I'll add a new hashmap :)

Thanks :-)

> But while we're here,
> what are your thoughts about moving submodule-specific to submodule.c?
> That includes the hashmap, get_submodule_ref_store() and all other
> submodule stuff, to submodule.c where submodule-specific stuff stays.
> The same for worktree stuff, to worktree.c. That keeps refs.c to core
> refs business. And the hashmap can be used for more than just refs.
> For example, submodule has configs, and worktree also has bunch of
> other stuff that I would like to just cache and not readdir() and
> parse every time.

I can't think of any objections, though there might be some devils in
the details. I assume you mean that the submodule-specific hashmap
wouldn't be `{name -> struct ref_store}` anymore, but rather `{name ->
struct submodule}`, and `struct submodule` would hold the associated
`ref_store`? That sounds very reasonable.

Instead of moving all of the `for_each_*_submodule()` functions over, I
encourage you to consider getting rid of them entirely and let the
end-users call the `refs_for_each_*()` versions of the functions. Again,
I'm not sure that there won't be friction in doing so, but it seems like
it's worth a try.

Michael