Hi,
On Thu, Apr 8, 2010 at 12:45 PM, Jeff King <peff@xxxxxxxx> wrote:
> On Thu, Apr 08, 2010 at 11:58:03AM +0800, Tay Ray Chuan wrote:
>
>> Instead of breaking execution when no remote (as specified in the
>> variable dest) is specified when git-ls-remote is invoked, continue on
>> and let remote_get() handle it.
>>
>> That way, we are able to use the default remote (by default, "origin"),
>> as git-fetch, git-push, and others, do.
>>
>> While we're at it, die with a more interesting message ("Where do you
>> want to..."), as git-fetch does, instead of the plain usage help.
>
> I don't really see a problem with this. The current behavior produces an
> error, so it is not as if we are breaking somebody's workflow, and the
> only sensible default is the same one used by the other commands.
I'm trying to make it behave like the other commands that deal with
remotes, such as git-fetch and git-push; when they are run without any
arguments, they default to "origin" or branch.<name>.remote.
Assuming that you and I are talking about the same "other commands",
than the default isn't an issue; the rules used to determine the
default remote is done in remote_get(), so they are similar.
>> + if (!remote)
>> + die("Where do you want to list from today?");
>
> Heh.
Do you think this is too light-hearted for ls-remote's role? If so,
I'll just revert back exiting with the usage text.
>> +test_expect_success 'dies with message when no remote specified and no default remote found' '
>> +
>> + !(git ls-remote >actual 2>&1) &&
>> + test_cmp exp actual
>
> Use test_must_fail?
Noted.
>> +cat >exp <<EOF
>> +fatal: 'refs*master' does not appear to be a git repository
>> +fatal: The remote end hung up unexpectedly
>> +EOF
>> +test_expect_success 'confuses pattern as remote when no remote specified' '
>> + #
>> + # Although ugly, this behaviour is akin to the confusion of refspecs for
>> + # remotes by git-fetch and git-push, eg:
>> + #
>> + # $ git fetch branch
>> + #
>> +
>> + # We could just as easily have used "master"; the "*" emphasizes its
>> + # role as a pattern.
>> + !(git ls-remote refs*master >actual 2>&1) &&
>> + test_cmp exp actual
>> +
>> +'
>
> This seems like a very odd thing to be testing. Should you not instead
> test that "git ls-remote $foo" still treats $foo as a remote and lists
> it, which is what we really care about?
There are already existing tests to test "git ls-remote $foo" (unless
you mean '$' to have a special significance, like '*' does).
The test was to let current and future git hackers/users know that the
usage of <pattern> as the remote by git-ls-remote ("<pattern> does not
appear to be a git repository") is indeed expected behaviour.
--
Cheers,
Ray Chuan
--
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