From: Eric Sunshine <sunshine@sunshineco.com>
Date: Wed, 29 Sep 2021 06:43:54 GMT
Subject: Re: [PATCH v2 0/3] Use default values from settings instead of config
Message-ID: <CAPig+cS7PF=F0XWO-EyVz_KuLnbnhQi3N01gyozSv99PmB_1RA@mail.gmail.com>
In-Reply-To: <kl6lh7e53nnn.fsf@chooglen-macbookpro.roam.corp.google.com>

On Mon, Sep 27, 2021 at 1:59 PM Glen Choo <chooglen@google.com> wrote:
> I wasn't clear about this in the original message, but I think this is
> ready to merge in its current form. I'd love to hear from reviewers who
> can poke holes in that :)
>
> Cc Eric and Taylor who have given the most review on v1 (thanks again!).
> I believe I've addressed your comments and I'd be interesting in hearing
> your thoughts on v2.

I re-read the entire thread and left a few comments on the v2 patches.
One or two of the comments might be actionable (i.e. deserve a
re-roll), but most are probably subjective.

Oh, I forgot to mention in my review of [3/3] that in:

    In the case where core.multiPackIndex is not set in the config, the auto
    condition would fail, causing the incremental-repack task to not be
    run. Because we always want to consider the default values, we should
    just always just use the_repository->settings.

you can probably drop one or both "just"s.