js/bisect-in-c, was Re: What's cooking in git.git (Aug 2022, #05; Mon, 15)

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Aug 16, 2022, 08:56 UTC
Message-ID: <p053rrpq-17q7-pnrs-3794-o04ro1445s5s@tzk.qr>
In-Reply-To: <xmqq5yityzcu.fsf@gitster.g>

Hi Junio,

On Mon, 15 Aug 2022, Junio C Hamano wrote:

Show 23 quoted lines

> * js/bisect-in-c (2022-06-27) 16 commits
>  - bisect: no longer try to clean up left-over `.git/head-name` files
>  - bisect: remove Cogito-related code
>  - Turn `git bisect` into a full built-in
>  - bisect: move even the command-line parsing to `bisect--helper`
>  - bisect: teach the `bisect--helper` command to show the correct usage strings
>  - bisect--helper: return only correct exit codes in `cmd_*()`
>  - bisect--helper: move the `BISECT_STATE` case to the end
>  - bisect--helper: make `--bisect-state` optional
>  - bisect--helper: align the sub-command order with git-bisect.sh
>  - bisect--helper: using `--bisect-state` without an argument is a bug
>  - bisect--helper: really retire `--bisect-autostart`
>  - bisect--helper: really retire --bisect-next-check
>  - bisect--helper: retire the --no-log option
>  - bisect: avoid double-quoting when printing the failed command
>  - bisect run: fix the error message
>  - bisect: verify that a bogus option won't try to start a bisection
>
>  Final bits of "git bisect.sh" have been rewritten in C.
>
>  Expecting a (hopefully final) reroll.
>  cf. <20627.86ilolhnnn.gmgdl@evledraar.gmail.com>
>  source: <pull.1132.v4.git.1656354677.gitgitgadget@gmail.com>

I had another look at the thread and did not see any feedback that focuses on the actual scope of the patch series. Conversions from scripted parts of Git to built-ins are always a bit finicky (and hard to review, I admit).

Therefore I would like to move the status to "needs review".

I do not think that there are any major issues with it (Ævar's feedback notwithstanding, it focuses on tangents that should be addressed after the conversion, to avoid losing focus), but I would love to see a thorough review of the conversion to avoid obvious regressions like the one in the built-in interactive `add` I had to fix recently.

Ciao, Dscho