From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: Elijah Newren <newren@gmail.com>, Git Mailing List <git@vger.kernel.org>
Subject: Re: js/bisect-in-c, was Re: What's cooking in git.git (Aug 2022, #05; Mon, 15)
Date: Fri, 19 Aug 2022 13:04:45 +0200 (CEST) [thread overview]
Message-ID: <snn8r5pn-rr6n-376r-s663-92s81pos077p@tzk.qr> (raw)
In-Reply-To: <xmqq35duvdow.fsf@gitster.g>
Hi Junio,
On Wed, 17 Aug 2022, Junio C Hamano wrote:
> Elijah Newren <newren@gmail.com> writes:
>
> > Ah, gotcha. My impression was that the exit codes did match what the
> > previous shell code had done, but didn't match what other builtins
> > usually return. Perhaps I misread those discussion comments.
>
> Or perhaps I did ;-)
The exit codes before and after this patch series are a red herring. The
_current_ code prints this when calling `git bisect start -h`:
$ git bisect start -h
usage: git bisect--helper --bisect-reset [<commit>]
or: git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]
or: git bisect--helper --bisect-start [--term-{new,bad}=<term> --term-{old,good}=<term>] [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]
or: git bisect--helper --bisect-next
or: git bisect--helper --bisect-state (bad|new) [<rev>]
or: git bisect--helper --bisect-state (good|old) [<rev>...]
or: git bisect--helper --bisect-replay <filename>
or: git bisect--helper --bisect-skip [(<rev>|<range>)...]
or: git bisect--helper --bisect-visualize
or: git bisect--helper --bisect-run <cmd>...
--bisect-reset reset the bisection state
--bisect-next-check check whether bad or good terms exist
--bisect-terms print out the bisect terms
--bisect-start start the bisect session
--bisect-next find the next bisection commit
--bisect-state mark the state of ref (or refs)
--bisect-log list the bisection steps so far
--bisect-replay replay the bisection process from the given file
--bisect-skip skip some commits for checkout
--bisect-visualize visualize the bisection
--bisect-run use <cmd>... to automatically bisect
--no-log no log for BISECT_WRITE
Notice how this talks about `bisect--helper` and about `--bisect-reset`.
Also, the _current_ code exits with code 0 when calling `git bisect -h`.
This has been the case even as far back as v2.25.1, and possibly even
longer.
Given these issues, I was mistakenly assuming that it would be okay to
postpone these problems that are exclusively related to incorrect
invocation of `git bisect`, and that it would make sense to focus on the
conversion from shell code to C in _this_ patch series, and take care of
these problems afterwards, instead of hodgepodging fixes for them into the
same patch series as the conversion to C, the latter being hard enough to
review as it stands, so much so that it received only a single high
quality review.
But I see that you somehow got the idea that the review that lacked
attention to the common code path somehow was a valid review and you
somehow got it in your mind that this was valid feedback and that the
patch series needs to be reworked so that it _also_ addresses issues that
have been broken _before_ it.
Fine.
I'll try to get to it next week. It does leave a foul taste that we're not
separating concerns properly in the Git project, but block a patch series
that has a specific, already large scope, just because one reviewer wants
it to have another scope and for some reason that now must be the scope of
the patch series.
Ciao,
Dscho
next prev parent reply other threads:[~2022-08-19 11:06 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-15 21:24 What's cooking in git.git (Aug 2022, #05; Mon, 15) Junio C Hamano
2022-08-16 8:56 ` js/bisect-in-c, was " Johannes Schindelin
2022-08-17 1:26 ` Elijah Newren
2022-08-17 6:27 ` Johannes Schindelin
2022-08-17 18:57 ` Junio C Hamano
2022-08-17 19:24 ` Elijah Newren
2022-08-17 20:05 ` Junio C Hamano
2022-08-19 11:04 ` Johannes Schindelin [this message]
2022-08-19 14:40 ` Ævar Arnfjörð Bjarmason
2022-08-16 16:00 ` sy/mv-out-of-cone (was Re: What's cooking in git.git (Aug 2022, #05; Mon, 15)) Victoria Dye
2022-08-16 16:16 ` Junio C Hamano
2022-08-17 2:03 ` Shaoxuan Yuan
2022-08-17 0:36 ` cw/submodule-merge-messages (Was: " Elijah Newren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=snn8r5pn-rr6n-376r-s663-92s81pos077p@tzk.qr \
--to=johannes.schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=newren@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).