On Wed, Jun 6, 2012 at 8:49 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Jens Lehmann <Jens.Lehmann@xxxxxx> writes: > >> Am 03.06.2012 11:46, schrieb Jon Seymour: >>> @@ -959,19 +985,32 @@ cmd_sync() >>> while read mode sha1 stage sm_path >>> do >>> name=$(module_name "$sm_path") >>> - url=$(git config -f .gitmodules --get submodule."$name".url) >>> + # path from superproject origin repo to submodule origin repo >> >> This comment is misleading as it only describes part of the truth, in a lot >> of cases it'll just be an absolute URL of the submodule. Done. v9 3/4 also updates the header comments for resolve_relative_url to remove a similar misleading implication. >> >>> + module_url=$(git config -f .gitmodules --get submodule."$name".url) >> >> And I see no real value of renaming "url" to "module_url" here (but maybe >> that is just me). > > I tend to agree; there is no other kind of URL involved, and I do > not see a clear motivation behind this renaming. Renaming url to > module_url would not help much if it is to differenciate URLs to the > repositories of submodule and superproject, so that can't be it. > > In any case, I suspect that you would be involved in maintaining > this code in the long haul, so even if it were "just you", your > opinion counts. > This is reverted in v9. >> So I'd vote for dropping that comment and the "url" to "module_url" change. >> But apart from that and the issues Junio mentioned in his response this >> series is looking good to me. > > Thanks for looking this over. Yes, thank you all. jon. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html