Re: What's cooking in git.git (Jun 2017, #03; Mon, 5)

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 6, 2017, 01:10 UTC
Message-ID: <xmqq8tl5apkk.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kY2Z-fJYxczbzheu1hChLkKkdjEcDMwsP-hkN0TjUBotQ@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 19 quoted lines

>>  "git diff" has been taught to optionally paint new lines that are
>>  the same as deleted lines elsewhere differently from genuinely new
>>  lines.
>>
>>  Are we happy with these changes?
>
> I advertised this series e.g. for reviewing Brandons
> repo object refactoring series and used it myself to inspect
> some patches there[1]. I am certainly happy (but biased) with
> what we have available there.
>
> Jacob intended to use this series
> for review as well, but has given no opinion yet.
>
> You seemed to have used it for js/blame-lib?
>
> --
> Those patches had a wide reviewer audience cc'd,
> so I would think people are aware of this series.

I tried to, yes. I haven't had a chance to see how well the current iteration fares "does the externally-visible goal make sense?" test.

I do not think I saw a negative "an approach to show this kind of output would not be useful" reaction, so I assume at least people would want an alternative output format that would help reviewing a change that moves blocks of lines around.

In any case, that is not a review. A patch series wanting to do a good thing, and people agreeing that the externally visible effect it produces matches that good thing, is one thing. A review that makes sure the code achieves the externally visible effect well (e.g. without overly inefficient algorithm, without buffer overflows or underflows, off-by-ones, etc.) is another thing, and I haven't seen anybody going with fine toothed comb to do that kind of review, hence my "are we happy?" inquiry.