Re: [PATCH v2 1/3] bisect: add parameters to "filter_skipped"

[Junio C Hamano <gitster@pobox.com>]

Christian Couder <chriscool@tuxfamily.org> writes:

> because we will need to get more information from this function in
> some later patches.
>
> The new "int *count" parameter gives the number of commits left after
> the skipped commit have been filtered out.
>
> The new "int *skipped_first" parameter tells us if the first commit
> in the list has been skipped. Note that using this parameter also
> changes the behavior of the function if the first commit is indeed
> skipped. Because we assume that in this case we will want all the
> filtered commits, not just the first one, even if "show_all" is not
> set.

The way you use (*skipped_first == -1) as a marker to mean "we've already
checked more than one commit_list so even when we see a one to be skipped,
it won't be the first one" is unreadable, especially without explanation.
Worse yet, the above paragraph explains what the parameter does, but why
is it so special to skip the one that happens to be the first on the input
list, especially when one does not know how the list is sorted to begin
with.

I understand that the list is sorted by the "goodness" value, i.e. the one
that cuts the graph into closer-to-equal halves comes earlier in the list,
but still it is unclear why having to skip the best one is so special
compared to having to skip say the second best one, especially when you
imagine a case where the first two on the list are of equal "goodness"
value.