Hi Miriam,

On Mon, 20 Jan 2020, Miriam Rubio wrote:

> --- Changes since Tanushree’s pr117 sent patch series:
> https://public-inbox.org/git/pull.117.git.gitgitgadget@gmail.com) ---
>
> General changes
> ---------------
>
> * Rebase on master branch.
> * Improve commit messages.
> * Amend patch series titles.
> * Reorder commits: first clean-up/preparatory commits, squash or split
> commits.

Great!

> Specific changes
> ----------------
>
> [1/29] bisect--helper: convert `vocab_*` char pointers to char arrays
>
> * New patch to convert `vocab_bad` and `vocab_good` char pointers
> to char arrays

29 patches is _a lot_ to review. I would have preferred a series of
smaller patch series.

For example, the first three patches would have made for a fine "some
cleanups" patch series, from my point of view.

Also, as the mail's subject says "part 1", it would be good to have an
overview how this part fits into the overall story of converting `git
bisect` into a built-in.

Finally, it would be nice to have a link to a public repository with the
branch from which these mails were generated.

I will try to review this patch series in its entirety, but it will take
me a while.

Ciao,
Johannes

>
> --
>
> [2/29] bisect--helper: change `retval` to `res`
>
> * Replace one last variable `retval` to `res`.
>
> --
>
> [3/29] bisect: use the standard 'if (!var)' way to check for 0
>
> * New patch to use '!var' and make 'bisect.c' more consistent with the
> rest of the code
>
> --
>
> [4/29] run-command: make `exists_in_PATH()` non-static
>
> * Add comment before function declaration.
> * Move function declaration in `run-command.h`.
>
> --
>
> [6/29] bisect: libify `exit_if_skipped_commits` to `error_if_skipped*`
> and its dependents
>
> * Fix `mark_edges_uninteresting()` and `show_diff_tree()` calls after
> rebase on master.
>
> --
>
> [7/29] bisect: libify `bisect_checkout`
>
> * Fix `memcpy()` call after rebase on master.
> * Introduce `res` variable to return `bisect_checkout()` output.
> * Fix `get_commit_reference()` declaration after rebase on master.
>
> --
>
> [8/29] bisect: libify `check_merge_bases` and its dependents
>
> State: Previously sent
>
> * Fix `check_ancestors()` declaration after rebase on master.
> * Fix `get_bad_and_good_commits()` call after rebase on master.
>
> --
>
> [9/29] bisect: libify `check_good_are_ancestors_of_bad` and its
> dependents
>
> State: Previously sent
>
> * Fix `check_good_are_ancestors_of_bad()` declaration after rebase on
> master.
> * Fix `check_good_are_ancestors_of_bad()`, `bisect_next_all()`
> and `bisect_rev_setup()` calls after rebase on master.
>
> --
>
> [11/29] bisect: libify `bisect_next_all`
>
> State: Previously sent
>
> * Fix `show_diff_tree()` call after rebase on master.
>
> --
>
> [12/29] bisect--helper: reimplement `bisect_next` and `bisect_auto_next`
> shell functions in C
>
> * Fix `repo_init_revisions()` and `bisect_next_all()` calls after rebase
> on master.
> * Remove `goto` statement in `bisect_skipped_commits()`
>
> --
>
> [13/29] bisect--helper: finish porting `bisect_start()` to C
>
> * Change `return` statement instead of `die` in error handling.
> * Remove `goto` statements in `bisect_skipped_commits()`.
>
> --
>
> [21/29] bisect--helper: reimplement `bisect_replay` shell function in C
>
> * Add blank line in `get_next_word()`.
> * Remove `goto` statements in `bisect_replay()`.
>
> --
>
> [23/29] bisect--helper: use `res` instead of return in BISECT_RESET case
> option
>
> * New patch to split previous commit in two.
>
> --
>
> [26/29] bisect--helper: reimplement `bisect_skip` shell function in C
>
> State: Previously sent
>
> * Add blank line.
>
> --
>
> [28/29] bisect--helper: reimplement `bisect_visualize()`shell function
> in C
>
> New patch:
>
> * Reimplement the `bisect_visualize()` shell function in C.
> * Add `--bisect-visualize` subcommand.
> * Fix long code line.
>
> --
>
> [29/29] bisect--helper: reimplement `bisect_run` shell function in C
>
> New patch:
>
> * Reimplement the `bisect_run()` shell function in C.
> * Add `--bisect-run` subcommand.
> * Remove blank line.
>
> --
>
> Miriam Rubio (2):
>   bisect--helper: convert `vocab_*` char pointers to char arrays
>   bisect: use the standard 'if (!var)' way to check for 0
>
> Pranit Bauva (24):
>   run-command: make `exists_in_PATH()` non-static
>   bisect: libify `exit_if_skipped_commits` to `error_if_skipped*` and
>     its dependents
>   bisect: libify `bisect_checkout`
>   bisect: libify `check_merge_bases` and its dependents
>   bisect: libify `check_good_are_ancestors_of_bad` and its dependents
>   bisect: libify `handle_bad_merge_base` and its dependents
>   bisect: libify `bisect_next_all`
>   bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell
>     functions in C
>   bisect--helper: finish porting `bisect_start()` to C
>   bisect--helper: retire `--bisect-clean-state` subcommand
>   bisect--helper: retire `--next-all` subcommand
>   bisect--helper: reimplement `bisect_autostart` shell function in C
>   bisect--helper: reimplement `bisect_state` & `bisect_head` shell
>     functions in C
>   bisect--helper: retire `--check-expected-revs` subcommand
>   bisect--helper: retire `--write-terms` subcommand
>   bisect--helper: reimplement `bisect_log` shell function in C
>   bisect--helper: reimplement `bisect_replay` shell function in C
>   bisect--helper: retire `--bisect-write` subcommand
>   bisect--helper: use `res` instead of return in BISECT_RESET case
>     option
>   bisect--helper: retire `--bisect-autostart` subcommand
>   bisect--helper: retire `--bisect-auto-next` subcommand
>   bisect--helper: reimplement `bisect_skip` shell function in C
>   bisect--helper: retire `--check-and-set-terms` subcommand
>   bisect--helper: reimplement `bisect_visualize()`shell function in C
>
> Tanushree Tumane (3):
>   bisect--helper: change `retval` to `res`
>   bisect--helper: introduce new `decide_next()` function
>   bisect--helper: reimplement `bisect_run` shell function in C
>
>  bisect.c                 | 146 +++++---
>  builtin/bisect--helper.c | 776 +++++++++++++++++++++++++++++++++------
>  git-bisect.sh            | 279 +-------------
>  run-command.c            |   2 +-
>  run-command.h            |  11 +
>  5 files changed, 793 insertions(+), 421 deletions(-)
>
> --
> 2.21.1 (Apple Git-122.3)
>
>