All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Lehmann <Jens.Lehmann@web.de>
To: Jon Seymour <jon.seymour@gmail.com>
Cc: git@vger.kernel.org, gitster@pobox.com, phil.hord@gmail.com,
	ramsay@ramsay1.demon.co.uk, j6t@kdbg.org
Subject: Re: [PATCH v8 3/4] submodule: fix sync handling of some relative superproject origin URLs
Date: Tue, 05 Jun 2012 23:18:34 +0200	[thread overview]
Message-ID: <4FCE77AA.6060005@web.de> (raw)
In-Reply-To: <1338716810-9881-4-git-send-email-jon.seymour@gmail.com>

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.

> +		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).

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.

  parent reply	other threads:[~2012-06-05 21:24 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-03  9:46 [PATCH v8 0/4] submodule: improve support for relative superproject origin URLs Jon Seymour
2012-06-03  9:46 ` [PATCH v8 1/4] submodule: additional regression tests for relative URLs Jon Seymour
2012-06-03  9:46 ` [PATCH v8 2/4] submodule: document failure to handle relative superproject origin URLs Jon Seymour
2012-06-03  9:46 ` [PATCH v8 3/4] submodule: fix sync handling of some " Jon Seymour
2012-06-03 22:10   ` Junio C Hamano
2012-06-03 23:52     ` Jon Seymour
2012-06-05 21:18   ` Jens Lehmann [this message]
2012-06-05 22:49     ` Junio C Hamano
2012-06-06 11:09       ` Jon Seymour
2012-06-03  9:46 ` [PATCH v8 4/4] submodule: fix handling of superproject origin URLs like foo, ./foo and ./foo/bar Jon Seymour

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FCE77AA.6060005@web.de \
    --to=jens.lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=jon.seymour@gmail.com \
    --cc=phil.hord@gmail.com \
    --cc=ramsay@ramsay1.demon.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.