Re: [PATCH v2 15/15] pack-revindex: write multi-pack reverse indexes

From: Jonathan Tan <jonathantanmy@google.com>
Date: Mar 2, 2021, 18:40 UTC
Message-ID: <20210302184033.12305-1-jonathantanmy@google.com>
In-Reply-To: <01bd6a35c6c441a30a22a4c2d17e9cf53de6b148.1614193703.git.me@ttaylorr.com>

Show 15 quoted lines

> @@ -1018,6 +1080,14 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
>  
>  	finalize_hashfile(f, midx_hash, CSUM_FSYNC | CSUM_HASH_IN_STREAM);
>  	free_chunkfile(cf);
> +
> +	if (flags & MIDX_WRITE_REV_INDEX)
> +		ctx.pack_order = midx_pack_order(&ctx);
> +
> +	if (flags & MIDX_WRITE_REV_INDEX)
> +		write_midx_reverse_index(midx_name, midx_hash, &ctx);
> +	clear_midx_files_ext(the_repository, ".rev", midx_hash);
> +
>  	commit_lock_file(&lk);
>  
>  cleanup:

Any reason why we're using 2 separate "if" statements?

Other than that, this patch and patch 14 look good. Besides all my minor comments, I think the overall patch set is in good shape and ready to be merged. It's great that we could reuse some of the individual-pack reverse index concepts and code too.