From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 27 Jan 2016 19:12:38 GMT
Subject: Re: [PATCH v4 1/2] merge-file: let conflict markers match end-of-line style of the context
Message-ID: <alpine.DEB.2.20.1601272011570.2964@virtualbox>
In-Reply-To: <xmqqio2e996c.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Wed, 27 Jan 2016, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> Just in case it was unclear, none of the comment above means I want
> >> any part of the patch redone--I am happy with this patch as-is.
> >
> > Thanks for saying that... I was about to try to make things clearer, but I
> > could not think of a better term than "needs_cr".
> 
> I don't, either ;-).
> 
> The primary reason I respond with the "I find this a bit confusing
> but it probably is just me" (not just to this patch) is to give an
> example of a review comment that demonstrates to the others that the
> reviewer understood what is in the patch and the issues around the
> change better than a mere unsubstantiated "These look OK to me.",
> which does not tell us how carefully the proposed change was
> reviewed by the reviewer--such a review does not allow me to "trust
> the review that is already done by others" and apply the patches
> with minimum cursory scanning and I end up having to carefully read
> them myself.

Your response is also an indicator to me that future myself will find the
same code just as confusing as you did, though.

Maybe need_cr -> eol_is_crlf?

Ciao,
Dscho