On Thu, Apr 08, 2010 at 02:07:11PM +0800, Tay Ray Chuan wrote:
> > 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.
The commands I meant were push, fetch, and pull. I couldn't think of any
others.
> >> + 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.
Perhaps. In general that is not a helpful message to show to the user,
but it is very unlikely to be shown at all. You would have to have no
configured remote for your current branch, _and_ you would have had to
delete the config for the "origin" remote.
Fetch's message is equally hard to trigger, I think, which is perhaps
why nobody has complained about it yet.
> > 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).
No, I meant $foo as a variable, as you interpreted.
> 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.
I think documenting that in the commit message would be fine, but I
don't have a strong opinion. It seems like the existing "git ls-remote
$foo" tests would cover this by correctly interpreting $foo as a remote.
If you do keep it, it also should use test_must_fail.
Also, I note that you are testing for:
fatal: 'refs*master' does not appear to be a git repository
fatal: The remote end hung up unexpectedly
which is coming from two separate processes, which means the output may
have a race condition. I suspect we're OK because the second message is
triggered by closing the fd which must happen after the first message is
printed.
-Peff
--
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