Hi Pranit,

On 10/27/2017 05:06 PM, Pranit Bauva wrote:
> A big thanks to Stephan and Ramsay for patiently reviewing my series and
> helping me get this merged.

You're welcome. ;)

In addition to the things mentioned by the other reviewers, only a minor
thing:

> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 35d2105f941c6..12754448f7b6a 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
[...]> @@ -106,13 +112,48 @@ static void check_expected_revs(const char
**revs, int rev_nr)
>       }
>  }
>  
> +static int bisect_reset(const char *commit)
> +{
> +     struct strbuf branch = STRBUF_INIT;
> +
> +     if (!commit) {
> +             if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1)
> +                     return !printf(_("We are not bisecting.\n"));

This is weird; I had to look up the return value of printf first before
I could understand what you are doing ;) I think that it is meant as a
shortcut for

        printf(_("We are not bisecting.\n"));
        return 0;

but please also express it with these two lines. (Or what is the point
of returning a non-zero value only in the case when nothing could be
printed?)

Best,
Stephan

Reply via email to