Subject: Re: [PATCH v2 0/8] repack: introduce `--write-midx`

From: Junio C Hamano <[email-masked]>
Date: Wed, 15 Sep 2021 12:29:20 -0700

Junio C Hamano <[email-masked]> writes:

> But if it is the case, I'd step back a bit and further question if
> "else if" is a good construct to use here.  We'd return if .m passes
> midx_contains_pack() check, and another check based on .to_include
> gives us an orthogonal chance to return early, so two "if" statement
> that are independent sitting next to each other may have avoided
> such a bug from the beginning, perhaps?

OK, I went back and checked your response to a review in an earlier
round.  If .m and .to_include cannot be turned on at the same time,
then I think "else if" would express the intention more clearly.

But if we go that route, the whole "if ... else if" may deserve a
comment that explains why .m and .to_include are fundamentally and
inherently mutually exclusive.  In other words, is it possible if
future enhancement may want to pass both .m and .to_include to allow
the code path to check both conditions and return early?

Thanks.