Re: [PATCH 1/2] pack-objects: Teach --use-bitmap-index codepath to respect --local, --honor-pack-keep and --incremental
- From
Kirill Smelkov <kirr@nexedi.com>
- Date
- Aug 9, 2016, 19:29 UTC
- Message-ID
- <20160809192858.GA25822@teco.navytux.spb.ru>
- In-Reply-To
- <xmqqtwetc2bh.fsf@gitster.mtv.corp.google.com>
On Tue, Aug 09, 2016 at 09:52:18AM -0700, Junio C Hamano wrote:
Show 11 quoted lines
> 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.
I see, thanks for explaining. I used to read it a bit the other way; maybe it is just an environment difference.
Show 18 quoted lines
> > 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.Ok, let it be list_packed_objects().
Show 14 quoted lines
> > -# 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.Ok, makes sense. Both patches adjusted and will be reposted.
Thanks, Kirill