git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jakob Jarmar <jakob.j@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v3] stash: avoid recursive hard reset on submodules
Date: Sat, 12 Oct 2019 10:46:36 +0900
Message-ID: <xmqqsgnyenwj.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20191011222448.GA650182@OJAN-PAAVO.localdomain>

Jakob Jarmar <jakob.j@gmail.com> writes:

> diff --git a/t/t3906-stash-submodule.sh b/t/t3906-stash-submodule.sh
> index d7219d6f8f..83106fa958 100755
> --- a/t/t3906-stash-submodule.sh
> +++ b/t/t3906-stash-submodule.sh
> @@ -1,6 +1,6 @@
>  #!/bin/sh
>  
> -test_description='stash apply can handle submodules'
> +test_description='stash can handle submodules'

Good attention to the detail ;-)

> +setup_basic() {

Style.  SP on both sides of () in our shell scripts (as seen in the
existing shell function in the same file).

> +	git init sub &&
> +	(
> +		cd sub &&
> +		test_commit sub_file
> +	) &&
> +	git init main &&
> +	(
> +		cd main &&
> +		git submodule add ../sub &&
> +		test_commit main_file
> +	) &&
> +	test_when_finished "rm -rf main sub"

Have test_when_finished that removes main and sub _before_ you start
creating sub and main.  

When the &&-cascade breaks anywhere, the control may not even reach
your test_when_finished that registers the clean-up procedure.
Imagine "git init sub" succeeds but "git init main" somehow
fails---you still want to clean up "sub".

Other than that, looks reasonably well done.  

Thanks for working on this.


> +}
> +
> +test_expect_success 'stash push with submodule.recurse=true preserves dirty submodule worktree' '
> +	setup_basic &&
> +	(
> +		cd main &&
> +		git config submodule.recurse true &&
> +		echo "x" >main_file.t &&
> +		echo "y" >sub/sub_file.t &&
> +		git stash push &&
> +		test_must_fail git -C sub diff --quiet
> +	)
> +'
> +
> +test_expect_success 'stash push and pop with submodule.recurse=true preserves dirty submodule worktree' '
> +	setup_basic &&
> +	(
> +		cd main &&
> +		git config submodule.recurse true &&
> +		echo "x" >main_file.t &&
> +		echo "y" >sub/sub_file.t &&
> +		git stash push &&
> +		git stash pop &&
> +		test_must_fail git -C sub diff --quiet
> +	)
> +'
> +
>  test_done

  reply index

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-10 23:34 [PATCH] " Jakob Jarmar
2019-10-11  0:11 ` [PATCH v2] " Jakob Jarmar
2019-10-11  6:02   ` Junio C Hamano
2019-10-11 22:24     ` [PATCH v3] " Jakob Jarmar
2019-10-12  1:46       ` Junio C Hamano [this message]
2019-10-12 15:24         ` Jakob Jarmar
2019-10-12 15:38         ` [PATCH v4] " Jakob Jarmar

Reply instructions:

You may reply publically 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqqsgnyenwj.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jakob.j@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

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror http://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox