Re: [PATCH 1/3] rev-list --bisect: Move finding bisection into do_find_bisection.

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

Christian Couder <chriscool@tuxfamily.org> writes:

> This factorises some code and make a big function smaller.

I think the refactoring itself makes sense, especially where it
simplifies the clean-up of weight array in early-return
codepath.  But I have a couple of comments, though.

> +static struct commit_list *do_find_bisection(struct commit_list *list,
> +					     int nr, int *weights);
> +
>  /*
>   * zero or positive weight is the number of interesting commits it can
>   * reach, including itself.  Especially, weight = 0 means it does not

The comment whose top part we can see here talks about the magic
values -1 and -2 used while do_find_bisection() after the
refactoring does its work, and these magic values are never
visible to the calling function.  You should move the comment to
the top of do_find_bisection() as well.

Also this forward declaration is unwarranted.  A bottom-up
sequence to define do_find_bisection() first, then to define its
sole caller find_bisection() next is easier to read at least for
me.

The latter comment also applies to your other patch.