On Mon, Aug 22, 2022 at 9:19 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Elijah Newren <newren@xxxxxxxxx> writes:
>
[...]
> > ... In contrast, if the user did specify
> > --no-commit and the previous strategy succeeded, then we will continue
> > the loop. That seems rather inconsistent, since --no-commit should
> > not affect the choice of strategy.
>
> Yeah, exactly.
>
> > However, I missed two things in my reading. You are correct that I
> > missed the "<=" as opposed to "<" when I wrote my commit message,
> > though that turns out to not matter much due to the second thing. The
> > second thing I missed was part of the code at the beginning of the
> > loop:
> >
> > for (i = 0; !merge_was_ok && i < use_strategies_nr; i++) {
>
> Ahhhh, that explains it. We leave as soon as we find merge_was_ok
> so this patch is not necessary. There is nothing to fix. The
> original was fine as-is.
Right, there was no bug or even optimization opportunity in the
original, it was just confusing...as evidenced by the fact that we
both misread the code.
The fact that we both misread the code, though, suggests there is
something to fix: code readability.
