Re: Feature request: 'git bisect run' option to redundantly check start and end commits

From: Junio C Hamano <gitster@pobox.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: Felipe Contreras <felipe.contreras@gmail.com>,
	Ed Avis <ed.avis@qmaw.com>, git <git@vger.kernel.org>,
	"Miriam R." <mirucam@gmail.com>
Subject: Re: Feature request: 'git bisect run' option to redundantly check start and end commits
Date: Sat, 19 Dec 2020 09:31:35 -0800	[thread overview]
Message-ID: <xmqqh7ohpurs.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <CAP8UFD3AHZppV2puere=zKyRjHhywCYQcO6q4yU7jJCfF0HZJg@mail.gmail.com> (Christian Couder's message of "Sat, 19 Dec 2020 08:46:31 +0100")

Christian Couder <christian.couder@gmail.com> writes:

> On Fri, Dec 18, 2020 at 6:15 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>>
>> > There has been a lot of work over the years to port code from shell in
>> > git-bisect.sh to C in builtin/bisect--helper.c. So it would be nice,
>> > if you plan to implement this feature, if you could do it directly in
>> > builtin/bisect--helper.c.
>>
>> Yeah, I'm aware and I might be willing to do that--which probably would
>> require moving the whole of "bisect run" to C--if there was an
>> indication that such a patch would actually be merged.
>
> I think it would likely be merged, as it would just be finishing the
> porting git bisect to C saga.

I agree that the issue such a patch series would try to address
(i.e. "reimplement 'bisect run' in C") may be worth solving, and I
do not offhand see why it would become a reason of not merging.

But the "cause" is mere prerequisite to consider a particular
iteration of such a series for inclusion.  I think it is
irresponsible to say "would likely" before seeing any patch to base
our assessment on.

If the design is poorly done (e.g. it does not solve the problem it
aims to solve well, it makes it hard to explain to end users what is
going on [*1*], or makes a poor design choice and paints us in a
corner we cannot easily escape from, ...), or if the execution is
poorly done (e.g. it is unmaintainable, it breaks other things by
mistake, it makes future changes harder, ...), it would not be
possible for the project to take the series.

Until reviewers comments aimed to help by pointing out these defects
are addressed, that is.  And if the series stops getting updated to
address such defects, it may end up getting not merged at all.

[Footnote]

*1* This obviously applies only for a series that makes externally
    visible behaviour change.  "rewrite 'bisect run' in C" may not
    have end-user visible effect (and if it does, such a regression
    is a reason to say "this is not yet ready").