Hi Junio,
On Mon, 9 Jan 2017, Junio C Hamano wrote:
> I however think that the renaming of read_author_script() is totally
> backwards from maintainability's point of view.
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
