Thread (1 message) 1 message, 1 author, 2020-08-04

Re: [PATCH v4 0/5] Introduce --first-parent flag for git bisect

From: Junio C Hamano <hidden>
Date: 2020-08-04 22:19:16

Aaron Lipman [off-list ref] writes:
OK, here's take 4! Responding to Junio's feedback, first:
quoted
That function [cmd_bisect__helper()] is supposed to be a thin shim
layer whose only reason of its existence is to serve as an interface
with the scripted version of "git bisect".  When everything is
migrated from the shell script, cmd_bisect__helper() should disappear.
...
This is going backwards, as far as I can tell.  If anything, I'd
rather see cmd_bisect__helper() get fixed so that it does not parse
"--first-parent" (and similarly, "--no-checkout" that you imitated)
into first_parent_only (and no_checkout) variables and passed as
parameters to bisect_start().
Thanks for the explanation, Junio. (And for bearing with me while I
gain familiarity with git's codebase.) Now that I've taken some time
to examine how git-bisect.sh and cmd_bisect__helper() fit together,
the correct approach is much more clear.
I did notice that no_checkout variable in the top-level
cmd_bisect__helper() was convenient to have in the current code
structure, and I didn't think how it should be fixed deeply enough,
so for the purpose of this series, I do not mind if it is left
untouched.  It's just that it was disturbing to see that addition of
"--first-parent" wanted to add yet another variable to the
top-level, and that the new variable was totally unneeded.
As no_checkout was also passed to bisect_next_all() [via implicitly
checking for the existence of .git/BISECT_HEAD when calling
bisect_next() in git-bisect.sh], I've removed that parameter and
instead check for .git/BISECT_HEAD in bisect_next_all() to determine
whether no-checkout mode is on.

This means no-checkout mode can no longer be enabled by "git
bisect--helper --next-all --no-checkout"
Unlike "start", "next-all" is not really a command in the end-user's
mind ("git bisect next" is, though).  It merely is just an interim
"convenience" interface for the remaining part of the scripted "git
bisect" to call into.  As long as the caller can do the same thing
as before, it should be OK, I would think.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help