Re: [PATCH v2 1/7] bisect--helper: reimplement `bisect_log` shell function in C
From: Johannes Schindelin <hidden>
Date: 2021-01-18 15:04:45
Hi Miriam, On Mon, 21 Dec 2020, Miriam Rubio wrote:
From: Pranit Bauva <redacted> 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 [off-list ref] Mentored-by: Christian Couder [off-list ref] Mentored-by: Johannes Schindelin [off-list ref] Signed-off-by: Pranit Bauva <redacted> Signed-off-by: Tanushree Tumane <redacted> Signed-off-by: Miriam Rubio <redacted>
Good. I see this was originally sent as [PATCH 20/29] in https://lore.kernel.org/git/20200120143800.900-21-mirucam@gmail.com/ (local), 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:
quoted hunk ↗ jump to hunk
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