Re: [PATCH 2/4] sequencer: do not translate parameters to error_resolve_conflict()

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 19, 2022, 17:36 UTC
Message-ID: <xmqqfshsm8z1.fsf@gitster.g>
In-Reply-To: <06s6r3s7-27nn-1o9s-1n7p-5413284r8740@tzk.qr>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 5 quoted lines

>> Perhaps we should have the error_resolve_conflict() function take a
>> "enum replay_action" instead?
>
> We could do that. We could also just delete the sequencer code. It's just
> that both are a bad idea.

Sorry, but I do not quite understand this comment. You may think some parts of the sequencer code are a bad idea but I think overall it is eminently useful and usable enough that it does not make sense to "just delete the sequencer code"---if there are things we find bad ideas in there, we should fix them instead, no?

In any case, can you keep the conversation more civil? I have to say that between you two, you may by no means be the only one who is unnecessarily abrasive, but if you do not understand why the other side suggests a solution you feel you do not like, you can ask more constructively why they think it is a good idea, without assuming that they are doing so only to block you. Or explain why you think it is a bad idea by showing the consequences of their solution, e.g. "there are 20 callsites, among which only 1 has the enum readily available so it would be a lot of churn to give the other 19 the enum, even though the error helper may become simpler with a single switch() statement if we allow it to take an enum." or something (I know this function is called only from very few places, so 1 out of 20 is a totally made-up reasoning that would not apply in this case, but you get the idea).

Thanks.