Re: [PATCH v3 3/8] repack: refactor finishing pack-objects command

From: Taylor Blau <me@ttaylorr.com>
Date: Jul 25, 2023, 22:45 UTC
Message-ID: <ZMBQmrMo+1hDIJwW@nand.local>
In-Reply-To: <20230724085909.3831831-4-christian.couder@gmail.com>

On Mon, Jul 24, 2023 at 10:59:04AM +0200, Christian Couder wrote:

Show 6 quoted lines

> Create a new finish_pack_objects_cmd() to refactor duplicated code
> that handles reading the packfile names from the output of a
> `git pack-objects` command and putting it into a string_list, as well as
> calling finish_command().
>
> While at it, beautify a code comment a bit in the new function.

Everything here looks good to me. Thanks for cleaning this up into its own function and DRY-ing things up a little bit.

Show 16 quoted lines

> Signed-off-by: Christian Couder <chriscool@tuxfamily.org
> ---
>  builtin/repack.c | 70 +++++++++++++++++++++++-------------------------
>  1 file changed, 33 insertions(+), 37 deletions(-)
>
> diff --git a/builtin/repack.c b/builtin/repack.c
> index aea5ca9d44..96af2d1caf 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -696,6 +696,36 @@ static void remove_redundant_bitmaps(struct string_list *include,
>  	strbuf_release(&path);
>  }
>
> +static int finish_pack_objects_cmd(struct child_process *cmd,
> +				   struct string_list *names,
> +				   int local)

I'm glad to see "local" in the arguments list ;-). I think that the implementation came out nice and clean here.

Thanks, Taylor