Thread (4 messages) flat view 4 messages, 3 authors, 2017-04-21

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

From: Duy Nguyen <hidden>
Date: 2017-04-21 09:40:06

On Fri, Apr 21, 2017 at 1:42 PM, Michael Haggerty [off-list ref] wrote:
On 04/21/2017 08:32 AM, Michael Haggerty wrote:
quoted
[...]
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help