Hello Junio,
On 2024-04-05 18:44, Junio C Hamano wrote:
Dragan Simic <dsimic@xxxxxxxxxxx> writes:
Well, I actually wasn't lazy, but instead I wanted to give this patch
better chances to be accepted, by introducing as a small amount of
changes to the code as possible. This was a lesson not to take that
route, but to do "the right thing" instead.
The lesson you should be taking is a bit different here, though.
As we discussed, doing the separator correctly is harder and doing
the terminator correctly is much easier and less error prone. So if
we chose to do terminator semantics and punt on doing separator
semantics, the right thing could have been to adjust the existing
tests to make sure that the new output with unnecessary trailing
blank line is expected.
Either that, or we should fix the code not to break the current
expectation. Doing neither of these is not quite acceptable---
that is the lesson from this episode, I would think.
Of course, it was my mistake not to either ensure that the tests
still pass after the introduced changes, or to adjust the test(s)
accordingly.
However, making the vertical whitespace a separator also follows
the Git's general approach of not wasting the vertical screen
space whenever possible, so taking the separator route is actually
better, if you agree.
