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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 15, 2021, 19:29 UTC
Message-ID: <xmqqilz1wsbz.fsf@gitster.g>
In-Reply-To: <xmqqmtodwsnz.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 6 quoted lines

> 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.