Re: [PATCH v3 00/38] Teach the sequencer to act as rebase -i's backend

Hi Junio,

On Mon, 9 Jan 2017, Junio C Hamano wrote:

You stated elsewhere that converting a script into a builtin should focus on a faithful conversion.

The original code is:

	. "$author_script"

Granted, this *cannot* be converted faithfully without reimplementing a shell interpreter. So I did the next best thing: I converted it into code that reads a block of environment variable settings.

What you asked for is totally unreasonable: you ask me to make this conversion *even less faithful*.

What is worse: you argue from the "maintainability point of view", when it is pretty obvious that *adding validation that was not there before* can in no way make the code more maintainable, as it *adds new logic*.

And what is the worst: over all these discussions about a nothingburger (you simply cannot convince me that I should introduce validating code that has not been there before in the same patch series that simply tries to recreate existing functionality), the most important part of a code review was forgotten: to make sure that the changes are correct.

The worst direction a code review can take is to introduce regressions, and that is exactly what happened.

Ciao, Johannes