From: Jay Soffian <jaysoffian@gmail.com>
Date: Sat, 14 Feb 2009 18:35:03 GMT
Subject: Re: [PATCH] builtin-remote: better handling of multiple remote HEADs
Message-ID: <76718490902141035o5430707ck47cd72d9efe87318@mail.gmail.com>
In-Reply-To: <20090214175420.GA3457@coredump.intra.peff.net>

On Sat, Feb 14, 2009 at 12:54 PM, Jeff King <peff@peff.net> wrote:
>> +             if (opt_a)
>> +                     printf("%s/HEAD set to %s\n", argv[0], head_name);
>
> This was a surprise based on reading the commit message, but I think it
> is a sensible enhancement.

It seemed that when doing something "--automatically" it might be nice
to tell the user what we just did, but I'm confused why this was a
surprise.

>> +cat > test/expect <<EOF
>> +origin/HEAD set to master
>> +EOF
>> +
>> +test_expect_success 'set-head --auto' '
>> +     (cd test &&
>> +      git remote set-head --auto origin > output &&
>> +      git symbolic-ref refs/remotes/origin/HEAD &&
>> +     test_cmp expect output)
>> +'
>
> I had to read this test a few times to convince myself it was right,
> since you throw away the output of symbolic-ref. I think it makes more
> sense to just test the post-command state, which is what you actually
> care about (and then you are also not dependent on the human-readable
> output of "remote set-head"). I.e.:
>
> cat > test/expect <<EOF
> refs/remotes/origin/master
> EOF
>
> test_expect_success 'set-head --auto' '
>        (cd test &&
>         git remote set-head --auto origin &&
>         git symbolic-ref refs/remotes/origin/HEAD > output &&
>        test_cmp expect output)
> '

Right.

j.