* [PATCH] git-submodule: a small fix
@ 2011-09-26 22:00 Roy Liu
2011-09-27 3:22 ` Andrew Ardill
0 siblings, 1 reply; 3+ messages in thread
From: Roy Liu @ 2011-09-26 22:00 UTC (permalink / raw)
To: git
In git-submodule.sh, the "url" variable may contain a stale value from
the previous loop iteration, so clear it.
--- git-submodule.sh.orig 2011-09-26 17:50:45.000000000 -0400
+++ git-submodule.sh 2011-09-26 17:51:18.000000000 -0400
@@ -370,6 +370,8 @@
esac
git config submodule."$name".url "$url" ||
die "Failed to register url for submodule path '$path'"
+ else
+ url=""
fi
# Copy "update" setting when it is not set yet
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] git-submodule: a small fix
2011-09-26 22:00 [PATCH] git-submodule: a small fix Roy Liu
@ 2011-09-27 3:22 ` Andrew Ardill
2011-09-27 7:24 ` Jens Lehmann
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Ardill @ 2011-09-27 3:22 UTC (permalink / raw)
To: Roy Liu; +Cc: git
On 27 September 2011 08:00, Roy Liu <carsomyr@gmail.com> wrote:
> In git-submodule.sh, the "url" variable may contain a stale value from
> the previous loop iteration, so clear it.
>
> --- git-submodule.sh.orig 2011-09-26 17:50:45.000000000 -0400
> +++ git-submodule.sh 2011-09-26 17:51:18.000000000 -0400
> @@ -370,6 +370,8 @@
> esac
> git config submodule."$name".url "$url" ||
> die "Failed to register url for submodule path '$path'"
> + else
> + url=""
> fi
>
> # Copy "update" setting when it is not set yet
Perhaps a better commit description would be:
git-submodule: clear the url variable when not set to avoid using stale values
otherwise looks clean enough
Andrew
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] git-submodule: a small fix
2011-09-27 3:22 ` Andrew Ardill
@ 2011-09-27 7:24 ` Jens Lehmann
0 siblings, 0 replies; 3+ messages in thread
From: Jens Lehmann @ 2011-09-27 7:24 UTC (permalink / raw)
To: Roy Liu; +Cc: Andrew Ardill, git
Am 27.09.2011 05:22, schrieb Andrew Ardill:
> On 27 September 2011 08:00, Roy Liu <carsomyr@gmail.com> wrote:
>> In git-submodule.sh, the "url" variable may contain a stale value from
>> the previous loop iteration, so clear it.
>>
>> --- git-submodule.sh.orig 2011-09-26 17:50:45.000000000 -0400
>> +++ git-submodule.sh 2011-09-26 17:51:18.000000000 -0400
>> @@ -370,6 +370,8 @@
>> esac
>> git config submodule."$name".url "$url" ||
>> die "Failed to register url for submodule path '$path'"
>> + else
>> + url=""
>> fi
>>
>> # Copy "update" setting when it is not set yet
>
> Perhaps a better commit description would be:
>
> git-submodule: clear the url variable when not set to avoid using stale values
Yes, the commit description needs to describe what was changed (and you can
also drop the "git-", to start with "submodule:" is sufficient here). Also
it would be nice if the message would describe under what circumstances this
happens (how did you notice this problem?).
But I wonder if the patch does it the right way. While it fixes the basic
issue that "url" might not be set, I doubt it does what the user expects.
The place where the - sometimes uninitialized - variable "url" is used is
a few lines down:
say "$(eval_gettext "Submodule '\$name' (\$url) registered for path '\$path'")"
I doesn't make much sense to say "Submodule 'foo' () registered for path 'foo'"
here. Shouldn't "url" be set to "$(git config "submodule.$name.url")"? And
when looking at the if you added the else to it might make sense to set it
unconditionally before the if and then test "$url" there instead of adding an
extra else.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-09-27 7:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-26 22:00 [PATCH] git-submodule: a small fix Roy Liu
2011-09-27 3:22 ` Andrew Ardill
2011-09-27 7:24 ` Jens Lehmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).