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

["Miriam R." <mirucam@gmail.com>]

El jue., 30 ene. 2020 a las 16:04, Johannes Schindelin
(<Johannes.Schindelin@gmx.de>) escribió:
>
> Hi Miriam,
>
> On Tue, 28 Jan 2020, Miriam Rubio wrote:
>
> > These patches correspond to a first part of patch series
> > of Outreachy project "Finish converting `git bisect` from shell to C"
> > started by the interns Pranit Bauva and Tanushree Tumane
> > (https://public-inbox.org/git/pull.117.git.gitgitgadget@gmail.com) and
> > continued by Miriam Rubio.
> >
> > This first part are formed of preparatory/clean-up patches and all
> > `bisect.c` libification work.
> >
> > These patch series emails were generated from:
> > https://gitlab.com/mirucam/git/commits/git-bisect-work-part1
> >
> > It has to be noted that in this version 2 nothing has been done about a
> > reviewer suggestion of using enums for error codes, because there was no
> > consensus about using them by the reviewers.
>
> It is a pity, as "magic" constants tend to remain "magic" (and eventually
> "tragic" when they have not been addressed properly).
>
> However, this does not need to hold up your patch series.
>
> I noted a couple of rather minor things; It looks pretty good to me
> already, though, and I thank you very much for splitting this patch series
> off, so that only the libification remains. It made for a pleasant read.
>
Thank you very much for the review.
I hope to send another patch series version soon with the new changes
and suggestions :)

Best,
Miriam

> Thanks,
> Dscho
>
> >
> > --- Changes since v1 Finish converting git bisect to C part 1 patch series ---
> >
> > General changes
> > ---------------
> >
> > * Previous patch series version has been split in smaller groups
> > in order to facilitate revision and integration.
> > * Rebase on master branch: c7a6207591 (Sync with maint, 2020-01-27).
> > * Improve commit messages.
> >
> > Specific changes
> > ----------------
> >
> > [6/11] bisect: libify `exit_if_skipped_commits` to `error_if_skipped*`
> > and its dependents
> >
> > * Remove redundant sentences in commit message.
> > * Use `if (res < 0)` instead of `if (res)`.
> >
> > [8/11] bisect: libify `check_merge_bases` and its dependents
> >
> > * Remove redundant sentence in commit message.
> >
> > --
> >
> > [9/11] bisect: libify `check_good_are_ancestors_of_bad` and its
> > dependents
> >
> > * Remove redundant sentences in commit message.
> > * Return in `if (!current_bad_oid)` condition.
> >
> > --
> >
> > [10/11] bisect: libify `handle_bad_merge_base` and its dependents
> >
> > * Remove redundant sentence in commit message.
> >
> > --
> >
> > [11/11] bisect: libify `bisect_next_all`
> >
> > * Remove redundant sentence in commit message.
> > * Add return codes explanations in `bisect.h`.
> >
> > --
> >
> > 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 (7):
> >   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`
> >
> > Tanushree Tumane (2):
> >   bisect--helper: change `retval` to `res`
> >   bisect--helper: introduce new `decide_next()` function
> >
> >  bisect.c                 | 136 +++++++++++++++++++++++++++------------
> >  bisect.h                 |  23 +++++++
> >  builtin/bisect--helper.c | 118 +++++++++++++++++----------------
> >  run-command.c            |   2 +-
> >  run-command.h            |  11 ++++
> >  5 files changed, 193 insertions(+), 97 deletions(-)
> >
> > --
> > 2.21.1 (Apple Git-122.3)
> >
> >