> On Tue, Mar 02, 2021 at 10:40:33AM -0800, Jonathan Tan wrote: > > > @@ -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? > > Yeah. This first if statement will turn into: > > if (flags & (MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP)) > > so that the pack order is computed in either case (since both the > existing write_midx_reverse_index() and the eventual write_midx_bitmap() > will be able to use the pack order). Ah, OK. That's what I was thinking of, but nice to have confirmation. Maybe write in the commit message that these are separated because in the future, one of the conditions will change.