Thread (2 messages) flat view 2 messages, 2 authors, 2016-06-15

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:34

Christian Couder [off-list ref] 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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help