All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Shourya Shukla <shouryashukla.oo@gmail.com>
Cc: git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH 1/3] t6025: modernize style
Date: Thu, 16 Jan 2020 22:42:04 +0100 (CET)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.2001162239400.46@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <20200116203622.4694-2-shouryashukla.oo@gmail.com>

Hi,

On Fri, 17 Jan 2020, Shourya Shukla wrote:

> The tests in `t6025-merge-symlinks.sh` were written a long time ago, and
> has a lot of style violations, including the mixed-use of tabs and spaces,
> missing indentations, and other shell script style violations. Update it to
> match the CodingGuidelines.
>
> Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
> ---

Sounds good. Just one nit:

>  t/t6025-merge-symlinks.sh | 97 ++++++++++++++++++++-------------------
>  1 file changed, 50 insertions(+), 47 deletions(-)
>
> diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
> index 433c4de08f..b9219af659 100755
> --- a/t/t6025-merge-symlinks.sh
> +++ b/t/t6025-merge-symlinks.sh
> @@ -10,52 +10,55 @@ if core.symlinks is false.'
>
>  . ./test-lib.sh
>
> -test_expect_success \
> -'setup' '
> -git config core.symlinks false &&
> -> file &&

Here, `file` is written as a 0-byte file, and...

> -git add file &&
> -git commit -m initial &&
> -git branch b-symlink &&
> -git branch b-file &&
> -l=$(printf file | git hash-object -t blob -w --stdin) &&
> -echo "120000 $l	symlink" | git update-index --index-info &&
> -git commit -m master &&
> -git checkout b-symlink &&
> -l=$(printf file-different | git hash-object -t blob -w --stdin) &&
> -echo "120000 $l	symlink" | git update-index --index-info &&
> -git commit -m b-symlink &&
> -git checkout b-file &&
> -echo plain-file > symlink &&
> -git add symlink &&
> -git commit -m b-file'
> -
> -test_expect_success \
> -'merge master into b-symlink, which has a different symbolic link' '
> -git checkout b-symlink &&
> -test_must_fail git merge master'
> -
> -test_expect_success \
> -'the merge result must be a file' '
> -test -f symlink'
> -
> -test_expect_success \
> -'merge master into b-file, which has a file instead of a symbolic link' '
> -git reset --hard && git checkout b-file &&
> -test_must_fail git merge master'
> -
> -test_expect_success \
> -'the merge result must be a file' '
> -test -f symlink'
> -
> -test_expect_success \
> -'merge b-file, which has a file instead of a symbolic link, into master' '
> -git reset --hard &&
> -git checkout master &&
> -test_must_fail git merge b-file'
> -
> -test_expect_success \
> -'the merge result must be a file' '
> -test -f symlink'
> +test_expect_success 'setup' '
> +	git config core.symlinks false &&
> +	touch file &&

... here we now use `touch` instead. We do prefer `>file` in this
instance, though, I think. At least we do not prohibit it.

Otherwise it looks very good!
Johannes

> +	git add file &&
> +	git commit -m initial &&
> +	git branch b-symlink &&
> +	git branch b-file &&
> +	l=$(printf file | git hash-object -t blob -w --stdin) &&
> +	echo "120000 $l	symlink" |
> +	git update-index --index-info &&
> +	git commit -m master &&
> +	git checkout b-symlink &&
> +	l=$(printf file-different | git hash-object -t blob -w --stdin) &&
> +	echo "120000 $l	symlink" |
> +	git update-index --index-info &&
> +	git commit -m b-symlink &&
> +	git checkout b-file &&
> +	echo plain-file >symlink &&
> +	git add symlink &&
> +	git commit -m b-file
> +'
> +
> +test_expect_success 'merge master into b-symlink, which has a different symbolic link' '
> +	git checkout b-symlink &&
> +	test_must_fail git merge master
> +'
> +
> +test_expect_success 'the merge result must be a file' '
> +	test -f symlink
> +'
> +
> +test_expect_success 'merge master into b-file, which has a file instead of a symbolic link' '
> +	git reset --hard &&
> +	git checkout b-file &&
> +	test_must_fail git merge master
> +'
> +
> +test_expect_success 'the merge result must be a file' '
> +	test -f symlink
> +'
> +
> +test_expect_success 'merge b-file, which has a file instead of a symbolic link, into master' '
> +	git reset --hard &&
> +	git checkout master &&
> +	test_must_fail git merge b-file
> +'
> +
> +test_expect_success 'the merge result must be a file' '
> +	test -f symlink
> +'
>
>  test_done
> --
> 2.20.1
>
>

  reply	other threads:[~2020-01-16 21:42 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-16 20:36 [PATCH 0/3] t6025: updating tests Shourya Shukla
2020-01-16 20:36 ` [PATCH 1/3] t6025: modernize style Shourya Shukla
2020-01-16 21:42   ` Johannes Schindelin [this message]
2020-01-16 20:36 ` [PATCH 2/3] t6025: replace pipe with redirection operator Shourya Shukla
2020-01-16 22:57   ` Junio C Hamano
2020-01-16 20:36 ` [PATCH 3/3] t6025: use helpers to replace test -f <path> Shourya Shukla
2020-01-16 22:58   ` Junio C Hamano
2020-01-17 20:44     ` [PATCH 0/3] t6025: amended changes after suggestions from the community Shourya Shukla
2020-01-17 20:44       ` [PATCH 1/3] t6025: modernize style Shourya Shukla
2020-01-17 21:15         ` Eric Sunshine
2020-01-17 21:28           ` Junio C Hamano
2020-01-17 20:44       ` [PATCH 2/3] t6025: replace pipe with redirection operator Shourya Shukla
2020-01-17 21:24         ` Eric Sunshine
2020-01-18  8:33           ` [PATCH 0/3] t6025: updating tests Shourya Shukla
2020-01-18  8:33             ` [PATCH 1/3] t6025: modernize style Shourya Shukla
2020-01-18  8:33             ` [PATCH 2/3] t6025: replace pipe with redirection operator Shourya Shukla
2020-01-18  8:33             ` [PATCH 3/3] t6025: use helpers to replace test -f <path> Shourya Shukla
2020-01-18  8:33             ` [PATCH v3 0/2] t025: amended changes after suggestions from the community Shourya Shukla
2020-01-18  8:33             ` [PATCH v3 1/2] t6025: modernize style Shourya Shukla
2020-01-21 21:57               ` Junio C Hamano
2020-01-18  8:33             ` [PATCH v3 2/2] t6025: use helpers to replace test -f <path> Shourya Shukla
2020-01-21 21:58               ` Junio C Hamano
2020-01-17 20:44       ` [PATCH 3/3] " Shourya Shukla
2020-01-16 21:46 ` [PATCH 0/3] t6025: updating tests Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=nycvar.QRO.7.76.6.2001162239400.46@tvgsbejvaqbjf.bet \
    --to=johannes.schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=shouryashukla.oo@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.