From: Jeff King <peff@peff.net>
Date: Thu, 15 Apr 2021 09:05:17 GMT
Subject: Re: [PATCH 2/2] repack: avoid loosening promisor pack objects in partial clones
Message-ID: <YHgBze9V7kq/TkqU@coredump.intra.peff.net>
In-Reply-To: <20210415010454.4077355-1-jonathantanmy@google.com>

On Wed, Apr 14, 2021 at 06:04:54PM -0700, Jonathan Tan wrote:

> > @@ -576,6 +577,10 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
> >  		repack_promisor_objects(&po_args, &names);
> >  
> >  		if (existing_packs.nr && delete_redundant) {
> > +			for_each_string_list_item(item, &names) {
> > +				strvec_pushf(&cmd.args, "--keep-pack=%s-%s.pack",
> > +					     packtmp_name, item->string);
> > +			}
> 
> Git style is to not have braces for single-statement loops.

It is, though given that for_each_string_list_item() is a weird macro
instead of a regular for-loop, IMHO it makes things more obvious to have
the braces.

(All the rest of your comments seemed quite good to me).

-Peff