All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jens Lehmann <Jens.Lehmann@web.de>
Cc: Git Mailing List <git@vger.kernel.org>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Heiko Voigt <hvoigt@hvoigt.net>,
	"W. Trevor King" <wking@tremily.us>
Subject: Re: [WIP/PATCH 3/9] Teach checkout the --[no-]recurse-submodules option
Date: Mon, 03 Feb 2014 14:56:54 -0800	[thread overview]
Message-ID: <xmqq8utrdcuh.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <52EFF2EA.9060709@web.de> (Jens Lehmann's message of "Mon, 03 Feb 2014 20:50:02 +0100")

Jens Lehmann <Jens.Lehmann@web.de> writes:

> +	set_config_update_recurse_submodules(
> +		parse_update_recurse_submodules_arg("--recurse-submodules-default",
> +						    recurse_submodules_default),
> +		recurse_submodules);

I think I saw these exact lines in another patch.  Perhaps the whole
thing can become a helper function that lets the caller avoid typing
the whole long strings that needs a strange/unfortunate line break? 

> diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
> index 06b18f8..bc3e1ca 100755
> --- a/t/t2013-checkout-submodule.sh
> +++ b/t/t2013-checkout-submodule.sh
> @@ -4,17 +4,57 @@ test_description='checkout can handle submodules'
>
>  . ./test-lib.sh
>
> +submodule_creation_must_succeed() {

Style: SP before (), i.e.

	submodule_creation_must_succeed () {

> +	# checkout base ($1)
> +	git checkout -f --recurse-submodules $1 &&
> +	git diff-files --quiet &&
> +	git diff-index --quiet --cached $1 &&

Please make it a habit to quote a parameter that is intended not to
be split at $IFS (e.g. write these as "$1" not as $1).  Otherwise
the reader has to wonder if this can be called with a "foo bar" and
the expects it to be split into two.

> +	# checkout target ($2)
> +	if test -d submodule; then

Style: no semicolons in standard control structure, i.e.

	if test -d submodule
	then

> +		echo change>>submodule/first.t &&

Style: SP before but not after redirection operator, i.e.

	echo foo >>bar

> +submodule_removal_must_succeed() {

Likewise.

> +	# checkout base ($1)
> +	git checkout -f --recurse-submodules $1 &&

Likewise.

> +	echo first > file &&

Likewise.

> +test_expect_success '"checkout --recurse-submodules" replaces submodule with files' '
> +	git checkout -f base &&
> +	git checkout -b replace_submodule_with_dir &&
> +	git update-index --force-remove submodule &&
> +	rm -rf submodule/.git .gitmodules &&
> +	git add .gitmodules submodule/* &&
> +	git commit -m "submodule replaced" &&
> +	git checkout -f base &&
> +	git submodule update -f &&
> +	git checkout --recurse-submodules replace_submodule_with_dir &&
> +	test -d submodule &&
> +	! test -e submodule/.git &&
> +	test -f submodule/first.t &&
> +	test -f submodule/second.t
> +'

Hmmmm.  Is it sufficient for these files to just exist, or do we
want to make sure they have expected contents?

Thanks.

  reply	other threads:[~2014-02-03 22:57 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-06 22:36 What's cooking in git.git (Jan 2014, #01; Mon, 6) Junio C Hamano
2014-01-06 23:16 ` Francesco Pretto
2014-01-06 23:32   ` Junio C Hamano
2014-01-06 23:45     ` Francesco Pretto
2014-01-07 17:49 ` Jens Lehmann
     [not found]   ` <xmqqvbxvekwv.fsf@gitster.dls.corp.google.com>
2014-02-03 19:47     ` [WIP/PATCH 0/9] v2 submodule recursive checkout] Jens Lehmann
2014-02-03 19:48       ` [WIP/PATCH 1/9] submodule: prepare for recursive checkout of submodules Jens Lehmann
2014-02-03 22:23         ` Junio C Hamano
2014-02-07 21:06           ` Jens Lehmann
2014-02-04  0:01         ` Jonathan Nieder
2014-02-07 21:01           ` Jens Lehmann
2014-02-03 19:49       ` [WIP/PATCH 2/9] Teach reset the --[no-]recurse-submodules option Jens Lehmann
2014-02-03 22:40         ` Junio C Hamano
2014-02-07 21:09           ` Jens Lehmann
2014-02-03 19:50       ` [WIP/PATCH 3/9] Teach checkout " Jens Lehmann
2014-02-03 22:56         ` Junio C Hamano [this message]
2014-02-07 21:12           ` Jens Lehmann
2014-02-03 19:50       ` [WIP/PATCH 4/9] Teach merge " Jens Lehmann
2014-02-03 23:01         ` Junio C Hamano
2014-02-07 21:23           ` Jens Lehmann
2014-02-07 22:00             ` Junio C Hamano
2014-02-07 22:08               ` W. Trevor King
2014-02-03 19:51       ` [WIP/PATCH 5/9] Teach bisect--helper " Jens Lehmann
2014-02-03 19:51       ` [WIP/PATCH 6/9] Teach bisect " Jens Lehmann
2014-02-03 20:04         ` W. Trevor King
2014-02-03 20:22           ` Jens Lehmann
2014-02-03 19:52       ` [WIP/PATCH 7/9] submodule: teach unpack_trees() to remove submodule contents Jens Lehmann
2014-02-03 20:10         ` W. Trevor King
2014-02-07 21:24           ` Jens Lehmann
2014-02-03 19:53       ` [WIP/PATCH 8/9] submodule: teach unpack_trees() to repopulate submodules Jens Lehmann
2014-02-03 19:54       ` [WIP/PATCH 9/9] submodule: teach unpack_trees() to update submodules Jens Lehmann
2014-02-03 20:19         ` W. Trevor King
2014-02-07 21:25           ` Jens Lehmann
2014-02-04  0:11         ` Duy Nguyen
2014-02-07 21:32           ` Jens Lehmann

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=xmqq8utrdcuh.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=hvoigt@hvoigt.net \
    --cc=jrnieder@gmail.com \
    --cc=wking@tremily.us \
    /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.