Re: [PATCH v2 16/21] bisect: make total number of commits global

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 16, 2016, 00:44 UTC
Message-ID: <xmqqh9f2756r.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1460294354-7031-17-git-send-email-s-beyer@gmx.net>

Stephan Beyer <s-beyer@gmx.net> writes:

Show 6 quoted lines

> The total number of commits in a bisect process is a property of
> the bisect process. Making this property global helps to make the code
> clearer.
>
> Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
> ---

After wondring about count++ vs nr, I re-read this one.

This patch is mislabled.

Making it global is a lessor, supposed-to-be-no-op change, but the bigger change is that the definition of "total" is silently changed.

The definition of mid-point was based on 'nr' in the original code, which counted only the tree-changing commits, and with this patch, it is based on 'total', which now only counts the tree-changing commits, so things are internally consistent, and the loop I was puzzled with, "while (counted < total)", would properly terminate.

Perhaps things become cleaner and easier to understand if this was split into two steps. One that changes the meaning of 'total' (and removes 'nr'), and the other that makes 'total' a global.

Thanks.