Re: [PATCH v3] bisect: create 'bisect_flags' parameter in find_bisection()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Harald,

On Sun, 15 Apr 2018, Harald Nordgren wrote:

> Make it possible to implement bisecting only on first parents or on
> merge commits by passing flags to find_bisection(), instead of just
> a 'find_all' boolean.
> 
> Signed-off-by: Harald Nordgren <haraldnordgren@xxxxxxxxx>

Very nice. Just two little suggestions:

> diff --git a/bisect.c b/bisect.c
> index a579b50884..19dac7491d 100644
> --- a/bisect.c
> +++ b/bisect.c
> @@ -254,7 +254,7 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n
>   */
>  static struct commit_list *do_find_bisection(struct commit_list *list,
>  					     int nr, int *weights,
> -					     int find_all)
> +					     unsigned int bisect_flags)

For flags, we frequently seem to use the type `unsigned` (which is the
same as `unsigned int`, just shorter).

>  {
>  	int n, counted;
>  	struct commit_list *p;
> @@ -313,7 +313,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
>  		clear_distance(list);
>  
>  		/* Does it happen to be at exactly half-way? */
> -		if (!find_all && halfway(p, nr))
> +		if (!(bisect_flags & BISECT_FIND_ALL) && halfway(p, nr))

Rather than expanding the short & sweet `find_all` to `(bisect_flags &
BISECT_FIND_ALL)` in three places in this function, I would rather just
define this local variable in the beginning:

	unsigned int find_all = bisect_flags & BISECT_FIND_ALL;

This would also reduce the size of the diff.

All in all, very nice!

Ciao,
Johannes

P.S.: I fully agree with Junio and eagerly await what comes next ;-)



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux