Re: [PATCH v3 07/24] multi-pack-index: expand test data

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 12, 2018, 18:02 UTC
Message-ID: <CAPig+cTEcLtvrQ25KDmc4Cmi9C2yCUpLCTWsK9q8SAiZt+N-Jw@mail.gmail.com>
In-Reply-To: <3e82dd51-2e30-291f-8753-c1d711459c1d@gmail.com>

On Thu, Jul 12, 2018 at 10:10 AM Derrick Stolee <stolee@gmail.com> wrote:

Show 8 quoted lines

> On 7/6/2018 12:36 AM, Eric Sunshine wrote:
> > There seems to be a fair bit of duplication in these tests which
> > create objects. Is it possible to factor out some of this code into a
> > shell function?
>
> In addition to the other small changes, this refactor in particular was
> a big change (but a good one). I'm sending my current progress in this
> direction, as I expect this can be improved.

I like the amount of code reduction. A couple minor comments...

Show 12 quoted lines

> +generate_objects () {
> +       i=$1
> +       iii=$(printf '%03i' $i)
> +       {
> +               test-tool genrandom "bar" 200 &&
> +               test-tool genrandom "baz $iii" 50
> +       } >wide_delta_$iii &&
> +       {
> +               test-tool genrandom "foo"$i 100 &&
> +               test-tool genrandom "foo"$(( $i + 1 )) 100 &&
> +               test-tool genrandom "foo"$(( $i + 2 )) 100
> +       } >>deep_delta_$iii &&

I think this should be: s/>>/>/

> +       echo $iii >file_$iii &&
> +       test-tool genrandom "$iii" 8192 >>file_$iii &&

And this: s/>>/>/

> +       git update-index --add file_$iii deep_delta_$iii wide_delta_$iii
> +}