Re: [PATCH v3 11/11] t6050-replace: use some long option names

2013-09-01 Thread Eric Sunshine 
On Sat, Aug 31, 2013 at 3:12 PM, Christian Couder
chrisc...@tuxfamily.org wrote:
 So that they are tested a litlle bit too.

 Signed-off-by: Christian Couder chrisc...@tuxfamily.org
 ---
  t/t6050-replace.sh | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

 diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
 index 0b07a0b..5dc26e8 100755
 --- a/t/t6050-replace.sh
 +++ b/t/t6050-replace.sh
 @@ -122,9 +122,9 @@ test_expect_success 'git replace listing and deleting' '
   test $HASH2 = $(git replace -l) 
   test $HASH2 = $(git replace) 
   aa=${HASH2%??} 
 - test $HASH2 = $(git replace -l $aa*) 
 + test $HASH2 = $(git replace --list $aa*) 
   test_must_fail git replace -d $R 
 - test_must_fail git replace -d 
 + test_must_fail git replace --delete 
   test_must_fail git replace -l -d $HASH2 

Is this sort of change a good idea? A person reading the code, but not
familiar with this particular patch, might not understand the
seemingly random choice of short versus long option usage. Such a
person might be led to wonder if there is some subtle difference
between the short and long forms, and then unnecessarily spend time
digging into the code and documentation in an attempt to figure it
out. Alternately, someone reading the code might be led to assume that
the person who added the tests was being sloppy.

Hopefully, t0040-parse-options should already be proof enough that
long options are correctly handled, but if you really want to test
them here too, it seems like a separate test would be more appropriate
than randomly changing short form options to long in various tests.

   git replace -d $HASH2 
   git show $HASH2 | grep A U Thor 
 @@ -147,7 +147,7 @@ test_expect_success 'git replace resolves sha1' '
   git show $HASH2 | grep O Thor 
   test_must_fail git replace $HASH2 $R 
   git replace -f $HASH2 $R 
 - test_must_fail git replace -f 
 + test_must_fail git replace --force 
   test $HASH2 = $(git replace)
  '

 @@ -278,7 +278,7 @@ test_expect_success 'replaced and replacement objects 
 must be of the same type'

  test_expect_success '-f option bypasses the type check' '
 git replace -f mytag $HASH1 2err 
 -   git replace -f HEAD^{tree} HEAD~1 2err 
 +   git replace --force HEAD^{tree} HEAD~1 2err 
 git replace -f HEAD^ $BLOB 2err
  '

 --
 1.8.4.rc1.31.g530f5ce.dirty
--
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 v3 11/11] t6050-replace: use some long option names

2013-09-01 Thread Christian Couder 
From: Philip Oakley philipoak...@iee.org

 So that they are tested a litlle bit too.
 s /litlle/little/

Thanks,
Christian.
--
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

[PATCH v3 11/11] t6050-replace: use some long option names

2013-08-31 Thread Christian Couder 
So that they are tested a litlle bit too.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 0b07a0b..5dc26e8 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -122,9 +122,9 @@ test_expect_success 'git replace listing and deleting' '
  test $HASH2 = $(git replace -l) 
  test $HASH2 = $(git replace) 
  aa=${HASH2%??} 
- test $HASH2 = $(git replace -l $aa*) 
+ test $HASH2 = $(git replace --list $aa*) 
  test_must_fail git replace -d $R 
- test_must_fail git replace -d 
+ test_must_fail git replace --delete 
  test_must_fail git replace -l -d $HASH2 
  git replace -d $HASH2 
  git show $HASH2 | grep A U Thor 
@@ -147,7 +147,7 @@ test_expect_success 'git replace resolves sha1' '
  git show $HASH2 | grep O Thor 
  test_must_fail git replace $HASH2 $R 
  git replace -f $HASH2 $R 
- test_must_fail git replace -f 
+ test_must_fail git replace --force 
  test $HASH2 = $(git replace)
 '
 
@@ -278,7 +278,7 @@ test_expect_success 'replaced and replacement objects must 
be of the same type'
 
 test_expect_success '-f option bypasses the type check' '
git replace -f mytag $HASH1 2err 
-   git replace -f HEAD^{tree} HEAD~1 2err 
+   git replace --force HEAD^{tree} HEAD~1 2err 
git replace -f HEAD^ $BLOB 2err
 '
 
-- 
1.8.4.rc1.31.g530f5ce.dirty

--
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 v3 11/11] t6050-replace: use some long option names

2013-08-31 Thread Philip Oakley 

From: Christian Couder chrisc...@tuxfamily.org
Subject: [PATCH v3 11/11] t6050-replace: use some long option names



So that they are tested a litlle bit too.

s /litlle/little/



Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
t/t6050-replace.sh | 8 
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 0b07a0b..5dc26e8 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -122,9 +122,9 @@ test_expect_success 'git replace listing and 
deleting' '

 test $HASH2 = $(git replace -l) 
 test $HASH2 = $(git replace) 
 aa=${HASH2%??} 
- test $HASH2 = $(git replace -l $aa*) 
+ test $HASH2 = $(git replace --list $aa*) 
 test_must_fail git replace -d $R 
- test_must_fail git replace -d 
+ test_must_fail git replace --delete 
 test_must_fail git replace -l -d $HASH2 
 git replace -d $HASH2 
 git show $HASH2 | grep A U Thor 
@@ -147,7 +147,7 @@ test_expect_success 'git replace resolves sha1' 
'

 git show $HASH2 | grep O Thor 
 test_must_fail git replace $HASH2 $R 
 git replace -f $HASH2 $R 
- test_must_fail git replace -f 
+ test_must_fail git replace --force 
 test $HASH2 = $(git replace)
'

@@ -278,7 +278,7 @@ test_expect_success 'replaced and replacement 
objects must be of the same type'


test_expect_success '-f option bypasses the type check' '
 git replace -f mytag $HASH1 2err 
- git replace -f HEAD^{tree} HEAD~1 2err 
+ git replace --force HEAD^{tree} HEAD~1 2err 
 git replace -f HEAD^ $BLOB 2err
'

--
1.8.4.rc1.31.g530f5ce.dirty




--
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