Christian Couder <chriscool@xxxxxxxxxxxxx> 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. - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
