On 04/21/2017 03:12 AM, Junio C Hamano wrote: > Stefan Beller <sbeller@xxxxxxxxxx> writes: > >> + Junio > > Just like Michael, I do not have strong enough opinion for or > against this patch to comment on it. > > I do agree with you that it would be a good longer-term direction to > use "submodule" for a "struct submodule" (i.e. submodule object), > and call a string that names a submodule either "submodule_name" or > "submodule_path" depending on how it names one, for maintainability > of the code. > > However I am not convinced that this patch is an improvement. Even > though parameter names in decls only serve documentation purpose and > it is even OK to only have type without name there, if we are going > to _have_ names, it would make sense to match them to the parameter > names actually used in the implementation. > > Updating these names used in refs.c would make a very noisy patch, > of course. But I am not sure if it is a good middle ground to avoid > that and to update only refs.h. One should never infer too much from my silence. As often as not it's because I'm simply busy with other things. But in this case Junio's right. I think it is a good idea to use argument names in declarations as documentation, and I also agree that it is a minus for the names in the declarations not to agree with the names in the definition. But the code that would have to be touched already has a lot of work going on in it, so conflicts would be likely. I've CCed Duy because I don't know whether he has more plans regarding submodule references. A natural followup to his recent work would be to add a feature to the `submodule` module that allows a caller to look up the `ref_store` object for the submodule. Then client code could use the `refs_for_each_ref(struct ref_store *, ...)` family of functions to access such references, and we could 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. Michael