All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Raghul Nanth A via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Derrick Stolee <derrickstolee@github.com>,
	Victoria Dye <vdye@github.com>,
	Raghul Nanth A <nanth.raghul@gmail.com>
Subject: Re: [PATCH v3] describe: enable sparse index for describe
Date: Thu, 30 Mar 2023 07:57:09 -0700	[thread overview]
Message-ID: <xmqq1ql64796.fsf@gitster.g> (raw)
In-Reply-To: <pull.1480.v3.git.git.1680155957146.gitgitgadget@gmail.com> (Raghul Nanth A. via GitGitGadget's message of "Thu, 30 Mar 2023 05:59:16 +0000")

"Raghul Nanth A via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Raghul Nanth A <nanth.raghul@gmail.com>
>
> Add usage and performance tests for describe

I think this sentence can easily go.

> Describe uses the index when it is run with --dirty flag, which uses the
> run_diff_index commmand. The command is sparse-index aware and hence we
> can just set the requires-full-index to false

End the sentence with full-stop.  More importantly, it would be
nicer to have something to substantiate the "this is sparse aware"
claim here, something like "since commit X" or "as shown in the test
tXXXX.YY".

Updated tests do look good, and the code change is of course minimal
and to the point, which is also good.

Thanks.

> diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh
> index 3242cfe91a0..db7887470f9 100755
> --- a/t/perf/p2000-sparse-operations.sh
> +++ b/t/perf/p2000-sparse-operations.sh
> @@ -43,6 +43,7 @@ test_expect_success 'setup repo and indexes' '
>  	done &&
>  
>  	git sparse-checkout init --cone &&
> +	git tag -a v1.0 -m "Final" &&
>  	git sparse-checkout set $SPARSE_CONE &&
>  	git checkout -b wide $OLD_COMMIT &&
>  
> @@ -125,5 +126,7 @@ test_perf_on_all git checkout-index -f --all
>  test_perf_on_all git update-index --add --remove $SPARSE_CONE/a
>  test_perf_on_all "git rm -f $SPARSE_CONE/a && git checkout HEAD -- $SPARSE_CONE/a"
>  test_perf_on_all git grep --cached --sparse bogus -- "f2/f1/f1/*"
> +test_perf_on_all git describe --dirty
> +test_perf_on_all 'echo >>new && git describe --dirty'
>  
>  test_done
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index 801919009e1..8bc35c51426 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -1514,6 +1514,24 @@ test_expect_success 'sparse-index is not expanded: stash' '
>  	ensure_not_expanded stash pop
>  '
>  
> +test_expect_success 'sparse-index is not expanded: describe' '
> +	init_repos &&
> +	# Add tag to be read by describe
> +	git -C sparse-index tag -a v1.0 -m "Version 1" &&
> +	ensure_not_expanded describe --dirty &&
> +	cp sparse-index-out sparse-index-dirty &&
> +	ensure_not_expanded describe &&
> +	cp sparse-index-out sparse-index-normal &&
> +	# Check describe has same output on clean tree
> +	test_cmp sparse-index-dirty sparse-index-normal &&
> +	echo "test" >>sparse-index/g &&
> +	ensure_not_expanded describe --dirty &&
> +	echo "v1.0-dirty" > actual &&
> +	# Check describe on dirty work tree
> +	test_cmp sparse-index-out actual &&
> +	ensure_not_expanded describe
> +'
> +
>  test_expect_success 'sparse index is not expanded: diff' '
>  	init_repos &&
>  
>
> base-commit: 27d43aaaf50ef0ae014b88bba294f93658016a2e

  reply	other threads:[~2023-03-30 14:57 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-27 14:20 [PATCH] describe: enable sparse index for describe Raghul Nanth A via GitGitGadget
2023-03-27 18:26 ` Junio C Hamano
2023-03-28 19:46   ` Derrick Stolee
2023-03-28 20:24     ` Junio C Hamano
2023-03-28 20:35       ` Derrick Stolee
2023-03-29 16:25 ` [PATCH v2] " Raghul Nanth A via GitGitGadget
2023-03-29 17:00   ` Junio C Hamano
2023-03-29 17:49   ` Victoria Dye
2023-03-29 18:27     ` Junio C Hamano
2023-03-30 16:10     ` Raghul Nanth
2023-04-03 16:37       ` Victoria Dye
2023-03-30  5:59   ` [PATCH v3] " Raghul Nanth A via GitGitGadget
2023-03-30 14:57     ` Junio C Hamano [this message]
2023-03-30 15:13       ` Junio C Hamano
2023-03-30 16:23     ` Victoria Dye
2023-03-31 15:43       ` [GSOC][PATCH] " Raghul Nanth A
2023-03-31 16:34         ` Junio C Hamano
2023-03-31 18:20     ` [GSOC][PATCH v4] " Raghul Nanth A
2023-04-03 16:34       ` Victoria Dye
2023-04-03 16:47       ` [GSOC][PATCH v5] " Raghul Nanth A
2023-04-03  7:35     ` [PATCH v4] " Raghul Nanth A

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=xmqq1ql64796.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=nanth.raghul@gmail.com \
    --cc=vdye@github.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.