public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Kirill Smelkov <kirr@nexedi•com>
Cc: "Jeff King" <peff@peff•net>, "Vicent Marti" <tanoku@gmail•com>,
	"Jérome Perrin" <jerome@nexedi•com>,
	"Isabelle Vallet" <isabelle.vallet@nexedi•com>,
	"Kazuhiko Shiozaki" <kazuhiko@nexedi•com>,
	"Julien Muchembled" <jm@nexedi•com>,
	git@vger•kernel.org
Subject: Re: [PATCH 1/2] pack-objects: Teach --use-bitmap-index codepath to respect --local, --honor-pack-keep and --incremental
Date: Tue, 09 Aug 2016 09:52:18 -0700	[thread overview]
Message-ID: <xmqqtwetc2bh.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20160809112123.GA14273@teco.navytux.spb.ru> (Kirill Smelkov's message of "Tue, 9 Aug 2016 14:21:27 +0300")

Kirill Smelkov <kirr@nexedi•com> writes:

> Would you please explain why we should not use touch if we do not care
> about timestamps? Simply style?

To help readers.

"touch A" forcess the readers wonder "does the timestamp of A
matter, and if so in what way?" and "does any later test care what
is _in_ A, and if so in what way?"  Both of them is wasting their
time when there is no reason why "touch" should have been used. 

> diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
> index cce95d8..44914ac 100755
> --- a/t/t5310-pack-bitmaps.sh
> +++ b/t/t5310-pack-bitmaps.sh
> @@ -8,16 +8,15 @@ objpath () {
>  }
>  
>  # show objects present in pack ($1 should be associated *.idx)
> -packobjects () {
> -	git show-index <$1 | cut -d' ' -f2
> +pack_list_objects () {
> +	git show-index <"$1" | cut -d' ' -f2
>  }

pack-list-objects still sounds as if you are packing "list objects",
though.  If you are listing packed objects (or objects in a pack),
list-packed-objects (or list-objects-in-pack) reads clearer and more
to the point, at least to me.

> -# hasany pattern-file content-file
> +# has_any pattern-file content-file
>  # tests whether content-file has any entry from pattern-file with entries being
>  # whole lines.
> -hasany () {
> -	# NOTE `grep -f` is not portable
> -	git grep --no-index -qFf $1 $2
> +has_any () {
> +	grep -qFf "$1" "$2"

Omitting "-q" would help those who have to debug breakage in this
test or the code that this test checks.  What test_expect_success
outputs is not shown by default, and running the test script with
"-v" would show them as a debugging aid.

Thanks.

  parent reply	other threads:[~2016-08-09 16:52 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-07 19:09 [PATCH] pack-objects: Use reachability bitmap index when generating non-stdout pack too Kirill Smelkov
2016-07-07 20:52 ` Jeff King
2016-07-08 10:38   ` Kirill Smelkov
2016-07-12 19:08     ` Kirill Smelkov
2016-07-13  8:30       ` Jeff King
2016-07-13  8:26     ` Jeff King
2016-07-13 10:52       ` Kirill Smelkov
2016-07-17 17:06         ` Kirill Smelkov
2016-07-19 11:29           ` Jeff King
2016-07-19 12:14             ` Kirill Smelkov
2016-07-25 18:40         ` Jeff King
2016-07-25 18:53           ` Jeff King
2016-07-27 20:15           ` Kirill Smelkov
2016-07-27 20:40             ` Junio C Hamano
2016-07-28 20:22               ` Kirill Smelkov
2016-07-28 21:18                 ` Junio C Hamano
2016-07-29  7:40                   ` Kirill Smelkov
2016-07-29  7:46                     ` [PATCH 1/2] pack-objects: Teach --use-bitmap-index codepath to respect --local, --honor-pack-keep and --incremental Kirill Smelkov
2016-08-01 18:17                       ` Junio C Hamano
2016-08-08 12:37                         ` Kirill Smelkov
2016-08-08 13:50                           ` Jeff King
2016-08-08 13:51                             ` Jeff King
2016-08-08 16:08                             ` Junio C Hamano
2016-08-08 19:06                             ` Junio C Hamano
2016-08-08 19:09                               ` Jeff King
2016-08-08 16:11                           ` Junio C Hamano
2016-08-08 18:19                             ` Kirill Smelkov
2016-08-08 18:57                               ` [PATCH v3] " Kirill Smelkov
2016-08-08 19:26                               ` [PATCH 1/2] " Junio C Hamano
2016-08-09 11:21                                 ` Kirill Smelkov
2016-08-09 11:25                                   ` [PATCH 1/2 v4] pack-objects: respect --local/--honor-pack-keep/--incremental when bitmap is in use Kirill Smelkov
2016-08-09 16:52                                   ` Junio C Hamano [this message]
2016-08-09 19:29                                     ` [PATCH 1/2] pack-objects: Teach --use-bitmap-index codepath to respect --local, --honor-pack-keep and --incremental Kirill Smelkov
2016-08-09 19:31                                       ` [PATCH 1/2 v5] pack-objects: respect --local/--honor-pack-keep/--incremental when bitmap is in use Kirill Smelkov
2016-08-18 17:52                                         ` Jeff King
2016-09-10 14:57                                           ` Kirill Smelkov
2016-09-10 15:01                                             ` [PATCH 1/2 v8] " Kirill Smelkov
2016-09-13  6:23                                               ` Junio C Hamano
2016-09-13  7:50                                                 ` Kirill Smelkov
2016-09-10 15:05                                             ` [PATCH] t/perf/run: Don't forget to copy config.mak.autogen & friends to build area Kirill Smelkov
2016-09-12 19:12                                               ` Junio C Hamano
2016-09-12 19:17                                                 ` Junio C Hamano
2016-09-12 23:10                                                   ` Junio C Hamano
2016-09-13  6:58                                                     ` Kirill Smelkov
2016-09-12 17:33                                             ` [PATCH 1/2 v5] pack-objects: respect --local/--honor-pack-keep/--incremental when bitmap is in use Junio C Hamano
2016-08-09 19:32                                       ` [PATCH 2/2 v7] pack-objects: use reachability bitmap index when generating non-stdout pack Kirill Smelkov
2016-08-18 18:06                                         ` Jeff King
2016-09-10 14:59                                           ` Kirill Smelkov
2016-09-10 15:01                                             ` [PATCH 2/2 v8] " Kirill Smelkov
2016-09-12 19:21                                             ` [PATCH 2/2 v7] " Junio C Hamano
2016-08-09 19:49                                       ` [PATCH 1/2] pack-objects: Teach --use-bitmap-index codepath to respect --local, --honor-pack-keep and --incremental Junio C Hamano
2016-07-29  7:47                     ` [PATCH v4 2/2] pack-objects: Teach it to use reachability bitmap index when generating non-stdout pack too Kirill Smelkov
2016-08-08 13:56                       ` Jeff King
2016-08-08 15:40                         ` Kirill Smelkov
2016-08-08 18:08                           ` Junio C Hamano
2016-08-08 18:13                             ` Kirill Smelkov
2016-08-08 18:28                               ` Junio C Hamano
2016-08-08 18:58                                 ` Kirill Smelkov
2016-08-08 18:55                           ` [PATCH v5] pack-objects: teach " Kirill Smelkov
2016-08-08 20:53                             ` Junio C Hamano
2016-08-09 11:21                               ` Kirill Smelkov
2016-08-09 11:26                                 ` [PATCH 2/2 v6] pack-objects: use reachability bitmap index when generating non-stdout pack Kirill Smelkov

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=xmqqtwetc2bh.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=isabelle.vallet@nexedi$(echo .)com \
    --cc=jerome@nexedi$(echo .)com \
    --cc=jm@nexedi$(echo .)com \
    --cc=kazuhiko@nexedi$(echo .)com \
    --cc=kirr@nexedi$(echo .)com \
    --cc=peff@peff$(echo .)net \
    --cc=tanoku@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox