Re: [PATCH] refs.h: rename submodule arguments to submodule_path

From: Duy Nguyen <pclouds@gmail.com>
Date: Apr 21, 2017, 09:38 UTC
Message-ID: <CACsJy8BJ2_omtR+LyE71JSSmHf9qVanbdgembqg_HJD6qNheNA@mail.gmail.com>
In-Reply-To: <8b2ef70c-4064-f726-8752-5bb0a898ee7a@alum.mit.edu>

On Fri, Apr 21, 2017 at 1:42 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:

Show 14 quoted lines

> 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