On Fri, Apr 21, 2017 at 1:42 PM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote: > On 04/21/2017 08:32 AM, Michael Haggerty wrote: >> [...] >> I've CCed Duy because I don't know whether he has more plans regarding >> submodule references [...] get rid of the >> `for_each_ref_submodule()` family of functions entirely. >> >> So perhaps the code that this patch touches won't be around long anyway. > > Oh yeah, he has done exactly that in his nd/prune-in-worktree patch > series. (I knew I'd seen that somewhere...) > > So it seems that the argument renaming has mostly been overtaken by > events, though even after Duy's patch series there are a few `const char > *submodule` arguments that could be renamed. Yeah. After that series, the only place that takes a submodule (path) is get_submodule_ref_store() (other functions are just helpers). Renaming it to submodule_path makes perfect sense. Johannes Sixt when reviewing that series also noticed the "path" nature of this "submodule" argument and suggested converting submodule[x] == '/' to is_dir_sep(submodule[i]) for that reason. At this point, I think Stefan even has the opportunity to reference/look up a submodule ref store by something other than a path (like "struct submodule *", or by name) if he wants to. But if you do that, maybe rename the current function to get_submodule_ref_store_by_path() before you add a new get_submodule_ref_store_by_whatever(). About ".. because I don't know whether he (Duy) has more plans regarding submodule references", I plan to convert "submodule[x] == '/'" to is_dir_sep(submodule[x]), but I think that's about it. I'm not involved much in submodule to see the direction it's heading. As far as refs code is concerned, a "struct ref_store *" is all it needs, regardless of how you obtain it. -- Duy