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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 19, 2020, 17:31 UTC
Message-ID: <xmqqh7ohpurs.fsf@gitster.c.googlers.com>
In-Reply-To: <CAP8UFD3AHZppV2puere=zKyRjHhywCYQcO6q4yU7jJCfF0HZJg@mail.gmail.com>

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

Show 14 quoted lines

> 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").