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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Aug 31, 2013 at 3:12 PM, Christian Couder
<chriscool@xxxxxxxxxxxxx> wrote:
> So that they are tested a litlle bit too.
>
> Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
> ---
>  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 2>err &&
> -       git replace -f HEAD^{tree} HEAD~1 2>err &&
> +       git replace --force HEAD^{tree} HEAD~1 2>err &&
>         git replace -f HEAD^ $BLOB 2>err
>  '
>
> --
> 1.8.4.rc1.31.g530f5ce.dirty
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]