mbox series

[00/11] Finish converting git bisect into a built-in

Message ID pull.1132.git.1643328752.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series Finish converting git bisect into a built-in | expand

Message

Philippe Blain via GitGitGadget Jan. 28, 2022, 12:12 a.m. UTC
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

Comments

Miriam R. Jan. 28, 2022, 8:54 a.m. UTC | #1
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
Johannes Schindelin Jan. 28, 2022, 12:42 p.m. UTC | #2
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
Miriam R. Jan. 28, 2022, 3:11 p.m. UTC | #3
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
Elijah Newren Jan. 30, 2022, 6:39 a.m. UTC | #4
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.
Elijah Newren Feb. 9, 2022, 4:41 a.m. UTC | #5
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.
Johannes Schindelin Feb. 22, 2022, 3:55 p.m. UTC | #6
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