From: Jon Seymour <jon.seymour@gmail.com>
Date: Wed, 06 Jun 2012 11:09:59 GMT
Subject: Re: [PATCH v8 3/4] submodule: fix sync handling of some relative superproject origin URLs
Message-ID: <CAH3AnrpFg=mUcrRMUhbFvShnXwpvJa5eE-VsqcJm0MNMz8C6zA@mail.gmail.com>
In-Reply-To: <7v7gvlnzlz.fsf@alter.siamese.dyndns.org>

On Wed, Jun 6, 2012 at 8:49 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jens Lehmann <Jens.Lehmann@web.de> 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.