From: Johannes Schindelin Date: Mon, 18 Jan 2021 15:02:17 GMT Subject: Re: [PATCH v2 1/7] bisect--helper: reimplement `bisect_log` shell function in C Message-ID: In-Reply-To: <20201221162743.96056-2-mirucam@gmail.com> Hi Miriam, On Mon, 21 Dec 2020, Miriam Rubio wrote: > From: Pranit Bauva > > Reimplement the `bisect_log()` shell function in C and also add > `--bisect-log` subcommand to `git bisect--helper` to call it from > git-bisect.sh . > > Using `--bisect-log` subcommand is a temporary measure to port shell > function to C so as to use the existing test suite. > > Mentored-by: Lars Schneider > Mentored-by: Christian Couder > Mentored-by: Johannes Schindelin > Signed-off-by: Pranit Bauva > Signed-off-by: Tanushree Tumane > Signed-off-by: Miriam Rubio Good. I see this was originally sent as [PATCH 20/29] in https://lore.kernel.org/git/20200120143800.900-21-mirucam@gmail.com/, but this version contains improvements: - It returns `BISECT_FAILED` in `bisect_log()` instead of -1 (and `BISECT_OK` instead of 0) - Instead of checking for `argc > 1` (which was wrong), it now verifies that no arguments were passed via `if (argc)` - It exits from the shell script appropriately when the helper failed Just one nit: > diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c > index 709eb713a3..1854377fa6 100644 > --- a/builtin/bisect--helper.c > +++ b/builtin/bisect--helper.c > @@ -938,6 +951,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) > N_("verify the next bisection state then checkout the next bisection commit"), BISECT_AUTO_NEXT), > OPT_CMDMODE(0, "bisect-state", &cmdmode, > N_("mark the state of ref (or refs)"), BISECT_STATE), > + OPT_CMDMODE(0, "bisect-log", &cmdmode, > + N_("output the contents of BISECT_LOG"), BISECT_LOG), If this is supposed to be a more permanent subcommand (and https://git-scm.com/docs/git-bisect#_bisect_log_and_bisect_replay suggests it might be), it would probably make more sense to describe the option in less implementation-specific detail. Maybe something like: list the bisection steps so far Ciao, Dscho