Re: [Outreachy][PATCH 00/29] Finish converting git bisect to C part 1

Hi Johannes,

El lun., 20 ene. 2020 a las 22:41, Johannes Schindelin
([off-list ref]) escribió:

Hi Miriam,

On Mon, 20 Jan 2020, Miriam Rubio wrote:

quoted

--- 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!

quoted

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.

Ok, I will discuss with my mentor how is the best way to split all this work.

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.

This is the link of the current branch:
https://gitlab.com/mirucam/git/commits/git-bisect-work2.8.2
I will add the public repository link on next cover-letters.
Thank you for the suggestion.

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

Thank you very much for reviewing.
Best,
Miriam

Ciao,
Johannes

quoted

--

[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)