Re: [PATCH v2 02/11] tests: at-combinations: check ref names directly

2013-05-07 Thread Felipe Contreras
On Wed, May 8, 2013 at 12:55 AM, Junio C Hamano  wrote:
> Felipe Contreras  writes:
>
>> Some committishes might point to the same commit, but through a
>> different ref, that's why it's better to check directly for the ref,
>> rather than the commit message.
>>
>> We can do that by calling rev-parse --symbolic-full-name, and to
>> differentiate the old from the new behavior we add an extra argument to
>> the check() helper.
>>
>> Signed-off-by: Ramkumar Ramachandra 
>> Signed-off-by: Felipe Contreras 
>> ---
>
> It is signed-off by Ram first but who is the author?  You, or him?

I am, but he sent the patch first.

>>  t/t1508-at-combinations.sh | 27 ---
>>  1 file changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh
>> index 46e3f16..bd2d2fe 100755
>> --- a/t/t1508-at-combinations.sh
>> +++ b/t/t1508-at-combinations.sh
>> @@ -4,9 +4,14 @@ test_description='test various @{X} syntax combinations 
>> together'
>>  . ./test-lib.sh
>>
>>  check() {
>> -test_expect_${3:-success} "$1 = $2" "
>> - echo '$2' >expect &&
>> - git log -1 --format=%s '$1' >actual &&
>> +test_expect_${4:-success} "$1 = $3" "
>> + if [ '$2' == 'commit' ]; then
>> + echo '$3' >expect &&
>> + git log -1 --format=%s '$1' >actual
>> + else
>> + echo '$3' >expect &&
>> + git rev-parse --symbolic-full-name '$1' >actual
>> + fi &&
>
> Move the echo outside of if, and match the overall style.

Right.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 02/11] tests: at-combinations: check ref names directly

2013-05-07 Thread Junio C Hamano
Felipe Contreras  writes:

> Some committishes might point to the same commit, but through a
> different ref, that's why it's better to check directly for the ref,
> rather than the commit message.
>
> We can do that by calling rev-parse --symbolic-full-name, and to
> differentiate the old from the new behavior we add an extra argument to
> the check() helper.
>
> Signed-off-by: Ramkumar Ramachandra 
> Signed-off-by: Felipe Contreras 
> ---

It is signed-off by Ram first but who is the author?  You, or him?

>  t/t1508-at-combinations.sh | 27 ---
>  1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh
> index 46e3f16..bd2d2fe 100755
> --- a/t/t1508-at-combinations.sh
> +++ b/t/t1508-at-combinations.sh
> @@ -4,9 +4,14 @@ test_description='test various @{X} syntax combinations 
> together'
>  . ./test-lib.sh
>  
>  check() {
> -test_expect_${3:-success} "$1 = $2" "
> - echo '$2' >expect &&
> - git log -1 --format=%s '$1' >actual &&
> +test_expect_${4:-success} "$1 = $3" "
> + if [ '$2' == 'commit' ]; then
> + echo '$3' >expect &&
> + git log -1 --format=%s '$1' >actual
> + else
> + echo '$3' >expect &&
> + git rev-parse --symbolic-full-name '$1' >actual
> + fi &&

Move the echo outside of if, and match the overall style.

echo '$3' >expect &&
if test '$2' = commit
then
git log ...
else
git rev-parse ...
fi &&


>   test_cmp expect actual
>  "
>  }
> @@ -35,14 +40,14 @@ test_expect_success 'setup' '
>   git branch -u upstream-branch new-branch
>  '
>  
> -check HEAD new-two
> -check "@{1}" new-one
> -check "@{-1}" old-two
> -check "@{-1}@{1}" old-one
> -check "@{u}" upstream-two
> -check "@{u}@{1}" upstream-one
> -check "@{-1}@{u}" master-two
> -check "@{-1}@{u}@{1}" master-one
> +check HEAD ref refs/heads/new-branch
> +check "@{1}" commit new-one
> +check "@{-1}" ref refs/heads/old-branch
> +check "@{-1}@{1}" commit old-one
> +check "@{u}" ref refs/heads/upstream-branch
> +check "@{u}@{1}" commit upstream-one
> +check "@{-1}@{u}" ref refs/heads/master
> +check "@{-1}@{u}@{1}" commit master-one
>  nonsense "@{u}@{-1}"
>  nonsense "@{1}@{u}"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html