From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org, "Beat Bolli" <dev+git@drbeat.li>,
"Jeff King" <peff@peff.org>,
"Eric Sunshine" <sunshine@sunshineco.com>,
"Torsten Bögershausen" <tboegi@web.de>
Subject: Re: [PATCH v4 1/2] merge-file: let conflict markers match end-of-line style of the context
Date: Wed, 27 Jan 2016 10:19:55 -0800 [thread overview]
Message-ID: <xmqqio2e996c.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1601270856150.2964@virtualbox> (Johannes Schindelin's message of "Wed, 27 Jan 2016 08:58:59 +0100 (CET)")
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.
next prev parent reply other threads:[~2016-01-27 18:20 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-22 17:01 [PATCH 0/2] Let merge-file write out conflict markers with correct EOLs Johannes Schindelin
2016-01-22 17:01 ` [PATCH 1/2] convert: add a helper to determine the correct EOL for a given path Johannes Schindelin
2016-01-22 18:47 ` Junio C Hamano
2016-01-22 19:04 ` Johannes Schindelin
2016-01-23 7:05 ` Torsten Bögershausen
2016-01-24 10:42 ` Johannes Schindelin
2016-01-23 6:12 ` Torsten Bögershausen
2016-01-24 10:41 ` Johannes Schindelin
2016-01-22 17:01 ` [PATCH 2/2] merge-file: consider core.crlf when writing merge markers Johannes Schindelin
2016-01-22 18:15 ` Junio C Hamano
2016-01-22 18:47 ` Johannes Schindelin
2016-01-22 19:08 ` Junio C Hamano
2016-01-24 10:44 ` Johannes Schindelin
2016-01-22 19:50 ` Eric Sunshine
2016-01-22 19:52 ` Eric Sunshine
2016-01-24 10:37 ` Johannes Schindelin
2016-01-24 18:26 ` Eric Sunshine
2016-01-25 7:02 ` Johannes Schindelin
2016-01-23 6:31 ` Torsten Bögershausen
2016-01-24 10:39 ` Johannes Schindelin
2016-01-22 17:52 ` [PATCH 0/2] Let merge-file write out conflict markers with correct EOLs Beat Bolli
2016-01-24 10:48 ` [PATCH v2 0/1] " Johannes Schindelin
2016-01-24 10:48 ` [PATCH v2 1/1] merge-file: let conflict markers match end-of-line style of the context Johannes Schindelin
2016-01-24 16:27 ` Torsten Bögershausen
2016-01-24 16:36 ` Torsten Bögershausen
2016-01-25 6:53 ` Johannes Schindelin
2016-01-25 19:45 ` Ramsay Jones
2016-01-26 8:54 ` Johannes Schindelin
2016-01-26 16:43 ` Ramsay Jones
2016-01-26 16:49 ` Johannes Schindelin
2016-01-24 22:09 ` [PATCH v2 0/1] Let merge-file write out conflict markers with correct EOLs Junio C Hamano
2016-01-25 7:25 ` Johannes Schindelin
2016-01-25 8:06 ` [PATCH v3 0/2] " Johannes Schindelin
2016-01-25 8:07 ` [PATCH v3 1/2] merge-file: let conflict markers match end-of-line style of the context Johannes Schindelin
2016-01-25 8:32 ` Torsten Bögershausen
2016-01-25 8:59 ` Johannes Schindelin
2016-01-25 20:12 ` Junio C Hamano
2016-01-26 9:04 ` Johannes Schindelin
2016-01-26 17:22 ` Junio C Hamano
2016-01-25 8:07 ` [PATCH v3 2/2] merge-file: ensure that conflict sections match eol style Johannes Schindelin
2016-01-25 9:20 ` Johannes Schindelin
2016-01-26 14:42 ` [PATCH v4 0/2] Let merge-file write out conflict markers with correct EOLs Johannes Schindelin
2016-01-26 14:42 ` [PATCH v4 1/2] merge-file: let conflict markers match end-of-line style of the context Johannes Schindelin
2016-01-26 18:23 ` Junio C Hamano
2016-01-26 21:14 ` Junio C Hamano
2016-01-27 7:58 ` Johannes Schindelin
2016-01-27 18:19 ` Junio C Hamano [this message]
2016-01-27 19:12 ` Johannes Schindelin
2016-01-27 19:32 ` Junio C Hamano
2016-01-28 7:55 ` Johannes Schindelin
2016-01-26 14:42 ` [PATCH v4 2/2] merge-file: ensure that conflict sections match eol style Johannes Schindelin
2016-01-27 16:37 ` [PATCH v5 0/2] Let merge-file write out conflict markers with correct EOLs Johannes Schindelin
2016-01-27 16:37 ` [PATCH v5 1/2] merge-file: let conflict markers match end-of-line style of the context Johannes Schindelin
2016-01-27 16:37 ` [PATCH v5 2/2] merge-file: ensure that conflict sections match eol style Johannes Schindelin
2016-01-27 20:22 ` [PATCH v5 0/2] Let merge-file write out conflict markers with correct EOLs Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqio2e996c.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=dev+git@drbeat.li \
--cc=git@vger.kernel.org \
--cc=peff@peff.org \
--cc=sunshine@sunshineco.com \
--cc=tboegi@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.