| Message ID | 20200128144026.53128-1-mirucam@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Finish converting git bisect to C part 1 | expand |
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. 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) > >
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) > > > >
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. --- 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(-)