Message ID | pull.1132.git.1643328752.gitgitgadget@gmail.com (mailing list archive) |
---|---|
Headers | show |
Series | Finish converting git bisect into a built-in | expand |
Hi Johannes, I had done the last commits to finish the porting of git bisect command since my Outreachy internship two years ago (https://gitlab.com/mirucam/git/-/commits/git-bisect-work-part5-v1), and I was planning to send them in the last patch series during this February. I have a very very little availability and I can not send the patch series frequently and I understand you are in hurry to finish this. I am sure your patch series will be more complete and better and you will do not need it, but if you want to use some of my commits(or parts) be my guest :). Regards, Miriam. El vie, 28 ene 2022 a las 1:12, Johannes Schindelin via GitGitGadget (<gitgitgadget@gmail.com>) escribió: > > After three GSoC/Outreachy students spent an incredible effort on this, it > is finally time to put a neat little bow on it. > > Johannes Schindelin (11): > bisect run: fix the error message > bisect--helper: retire the --no-log option > bisect--helper: really retire --bisect-next-check > bisect--helper: really retire `--bisect-autostart` > bisect--helper: align the sub-command order with git-bisect.sh > bisect--helper: make `--bisect-state` optional > bisect: move even the option parsing to `bisect--helper` > bisect--helper: using `--bisect-state` without an argument is a bug > Turn `git bisect` into a full built-in. > bisect: remove Cogito-related code > bisect: no longer try to clean up left-over `.git/head-name` files > > Makefile | 3 +- > bisect.c | 3 - > builtin.h | 2 +- > builtin/{bisect--helper.c => bisect.c} | 201 ++++++++++--------------- > git-bisect.sh | 84 ----------- > git.c | 2 +- > t/t6030-bisect-porcelain.sh | 1 - > 7 files changed, 84 insertions(+), 212 deletions(-) > rename builtin/{bisect--helper.c => bisect.c} (87%) > delete mode 100755 git-bisect.sh > > > base-commit: 89bece5c8c96f0b962cfc89e63f82d603fd60bed > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1132%2Fdscho%2Fbisect-in-c-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1132/dscho/bisect-in-c-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/1132 > -- > gitgitgadget
Hi Miriam, On Fri, 28 Jan 2022, Miriam R. wrote: > I had done the last commits to finish the porting of git bisect command > since my Outreachy internship two years ago ( > https://gitlab.com/mirucam/git/-/commits/git-bisect-work-part5-v1), and I > was planning to send them in the last patch series during this February. Oh sorry! I specifically went to look at your fork before starting to work on the patch series, but had not seen that branch. Now I updated my remote-tracking branches and see it as a new branch, so I am not sure whether I missed it or whether it simply wasn't there. All in all, we did very similar things. I have a slight preference for the more concise version that does not use any command modes (and therefore also does not duplicate the commands in the output of `git bisect -h`). Maybe you could find time in February to review my patch series? Not a big deal if you're too busy. Ciao, Dscho
El vie, 28 ene 2022 a las 13:42, Johannes Schindelin (<Johannes.Schindelin@gmx.de>) escribió: > > Hi Miriam, > > On Fri, 28 Jan 2022, Miriam R. wrote: > > > I had done the last commits to finish the porting of git bisect command > > since my Outreachy internship two years ago ( > > https://gitlab.com/mirucam/git/-/commits/git-bisect-work-part5-v1), and I > > was planning to send them in the last patch series during this February. > > Oh sorry! I specifically went to look at your fork before starting to work > on the patch series, but had not seen that branch. Now I updated my > remote-tracking branches and see it as a new branch, so I am not sure > whether I missed it or whether it simply wasn't there. Hi Johannes, the branch ..part5-v1, I think it is from the last month, but the commits were done like two years ago during my internship and they were also in this branch (https://gitlab.com/mirucam/git/-/commits/git-bisect-work4.7). But don’t worry. Of course your patch series will be way better. > > All in all, we did very similar things. I have a slight preference for the > more concise version that does not use any command modes (and therefore > also does not duplicate the commands in the output of `git bisect -h`). > > Maybe you could find time in February to review my patch series? Not a big > deal if you're too busy. > Haha, I could review it, but I am sure they will be perfect. Ciao, Miriam. > Ciao, > Dscho
On Fri, Jan 28, 2022 at 3:08 PM Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com> wrote: > > After three GSoC/Outreachy students spent an incredible effort on this, it > is finally time to put a neat little bow on it. > > Johannes Schindelin (11): > bisect run: fix the error message > bisect--helper: retire the --no-log option > bisect--helper: really retire --bisect-next-check > bisect--helper: really retire `--bisect-autostart` > bisect--helper: align the sub-command order with git-bisect.sh > bisect--helper: make `--bisect-state` optional > bisect: move even the option parsing to `bisect--helper` > bisect--helper: using `--bisect-state` without an argument is a bug > Turn `git bisect` into a full built-in. > bisect: remove Cogito-related code > bisect: no longer try to clean up left-over `.git/head-name` files > > Makefile | 3 +- > bisect.c | 3 - > builtin.h | 2 +- > builtin/{bisect--helper.c => bisect.c} | 201 ++++++++++--------------- > git-bisect.sh | 84 ----------- > git.c | 2 +- > t/t6030-bisect-porcelain.sh | 1 - > 7 files changed, 84 insertions(+), 212 deletions(-) > rename builtin/{bisect--helper.c => bisect.c} (87%) > delete mode 100755 git-bisect.sh I read through the series and couldn't spot any problems.
On Sat, Jan 29, 2022 at 10:39 PM Elijah Newren <newren@gmail.com> wrote: > > On Fri, Jan 28, 2022 at 3:08 PM Johannes Schindelin via GitGitGadget > <gitgitgadget@gmail.com> wrote: > > > > After three GSoC/Outreachy students spent an incredible effort on this, it > > is finally time to put a neat little bow on it. > > > > Johannes Schindelin (11): > > bisect run: fix the error message > > bisect--helper: retire the --no-log option > > bisect--helper: really retire --bisect-next-check > > bisect--helper: really retire `--bisect-autostart` > > bisect--helper: align the sub-command order with git-bisect.sh > > bisect--helper: make `--bisect-state` optional > > bisect: move even the option parsing to `bisect--helper` > > bisect--helper: using `--bisect-state` without an argument is a bug > > Turn `git bisect` into a full built-in. > > bisect: remove Cogito-related code > > bisect: no longer try to clean up left-over `.git/head-name` files > > > > Makefile | 3 +- > > bisect.c | 3 - > > builtin.h | 2 +- > > builtin/{bisect--helper.c => bisect.c} | 201 ++++++++++--------------- > > git-bisect.sh | 84 ----------- > > git.c | 2 +- > > t/t6030-bisect-porcelain.sh | 1 - > > 7 files changed, 84 insertions(+), 212 deletions(-) > > rename builtin/{bisect--helper.c => bisect.c} (87%) > > delete mode 100755 git-bisect.sh > > I read through the series and couldn't spot any problems. I re-read the series, taking a closer look. Spotted a few minor things (and left some comments) but the series looks pretty good to me. I think Dscho's on vacation, so we'll resume the discussion when he gets back.
Hi Elijah, On Tue, 8 Feb 2022, Elijah Newren wrote: > On Sat, Jan 29, 2022 at 10:39 PM Elijah Newren <newren@gmail.com> wrote: > > > > On Fri, Jan 28, 2022 at 3:08 PM Johannes Schindelin via GitGitGadget > > <gitgitgadget@gmail.com> wrote: > > > > > > After three GSoC/Outreachy students spent an incredible effort on this, it > > > is finally time to put a neat little bow on it. > > > > > > Johannes Schindelin (11): > > > bisect run: fix the error message > > > bisect--helper: retire the --no-log option > > > bisect--helper: really retire --bisect-next-check > > > bisect--helper: really retire `--bisect-autostart` > > > bisect--helper: align the sub-command order with git-bisect.sh > > > bisect--helper: make `--bisect-state` optional > > > bisect: move even the option parsing to `bisect--helper` > > > bisect--helper: using `--bisect-state` without an argument is a bug > > > Turn `git bisect` into a full built-in. > > > bisect: remove Cogito-related code > > > bisect: no longer try to clean up left-over `.git/head-name` files > > > > > > Makefile | 3 +- > > > bisect.c | 3 - > > > builtin.h | 2 +- > > > builtin/{bisect--helper.c => bisect.c} | 201 ++++++++++--------------- > > > git-bisect.sh | 84 ----------- > > > git.c | 2 +- > > > t/t6030-bisect-porcelain.sh | 1 - > > > 7 files changed, 84 insertions(+), 212 deletions(-) > > > rename builtin/{bisect--helper.c => bisect.c} (87%) > > > delete mode 100755 git-bisect.sh > > > > I read through the series and couldn't spot any problems. > > I re-read the series, taking a closer look. Spotted a few minor > things (and left some comments) but the series looks pretty good to > me. I think Dscho's on vacation, so we'll resume the discussion when > he gets back. I took some time off, and then had to focus on other things, so I only got back to this patch series today. Thank you so much for your helpful feedback! Ciao, Dscho