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