git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: [PATCH v3 0/4] Handle PuTTY (plink/tortoiseplink) even in GIT_SSH_COMMAND
Date: Wed, 01 Feb 2017 14:55:11 -0800	[thread overview]
Message-ID: <xmqqr33hmshc.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1702012311440.3496@virtualbox> (Johannes Schindelin's message of "Wed, 1 Feb 2017 23:17:54 +0100 (CET)")

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Compared to the correctness issue, these are much harder to spot by
>> the submitter alone, who focused so intensely to get his code
>> "correct".  The review process is of greater value to spot these
>> issues.
>
> We will never agree on this.

That's too bad.

> From my perspective, design, explanation and maintainability are a
> consequence of making it easy for reviewers to spot where the code is
> incorrect.
>
> And correctness is not covered by "the submitter tested this". Correctness
> includes all the corner cases, where the "many eyes make bugs shallow"
> really shines.
>
> I'd rather have reviewers find bugs than users.

I'd rather have submitters find bugs than reviewers.

> I will *never* be a fan of a review process that pushes correctness to a
> back seat (yes, it is much harder than spotting typos or lines longer than
> 80 columns per row, but the ultimate goal is to deliver value to the end
> user, not to make life easy for the maintainer).

Did I ever say correctness is pushed to a back seat?

I said that it is easier to spot correctness issues for you as a
submitter than other kinds of issues without outside help (and
implied that if you are a diligent contributor, you should aim for,
and you should be able to achieve, a patch series where correctness
issues do not need to be pointed out).  But other higher level
issues are harder for any submitter to spot (regardless of
experience and competence of the submitter), because one gets so
married to one's own code, design and worldview.  And that is why
"review is primarily to spot bugs" can never be a correct viewpoint.

A reviewee needs to be prepared to accept review comments on higher
level issues, even more readily than comments on correctness issues,
because it is too easy to be constrained by early decisions one has
already made while preparing a patch series and become blind to
bigger picture after staring one's own new code for number of hours.
Higher level issues can be more easily spotted by reviewers, whose
eyes are still fresh to the series.

      reply	other threads:[~2017-02-01 22:55 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-02 12:09 [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND Johannes Schindelin
2017-01-08  2:33 ` Junio C Hamano
2017-01-09  1:08   ` Junio C Hamano
2017-01-09  7:46     ` Johannes Schindelin
2017-01-09  9:28       ` Junio C Hamano
2017-01-09 11:13         ` Johannes Schindelin
2017-01-09 14:19           ` Junio C Hamano
2017-01-25 12:34             ` Johannes Schindelin
2017-01-25 22:35               ` Junio C Hamano
2017-01-25 22:37                 ` Junio C Hamano
2017-01-26 14:45                   ` Johannes Schindelin
2017-01-25 22:40                 ` Jeff King
2017-01-25 23:25                   ` Junio C Hamano
2017-01-26 12:01                 ` Johannes Schindelin
2017-01-26 14:51 ` [PATCH v2 0/3] Handle PuTTY (plink/tortoiseplink) even " Johannes Schindelin
     [not found] ` <cover.1485442231.git.johannes.schindelin@gmx.de>
2017-01-26 14:51   ` [PATCH v2 1/3] connect: handle putty/plink also " Johannes Schindelin
2017-01-26 14:51   ` [PATCH v2 2/3] connect: rename tortoiseplink and putty variables Johannes Schindelin
2017-01-26 14:52   ` [PATCH v2 3/3] connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config Johannes Schindelin
2017-01-26 19:27     ` Junio C Hamano
2017-01-27 10:35       ` Johannes Schindelin
2017-01-27 18:17       ` Junio C Hamano
2017-02-01 12:01         ` Johannes Schindelin
2017-02-01 16:53           ` Junio C Hamano
2017-02-01 11:57   ` [PATCH v3 0/4] Handle PuTTY (plink/tortoiseplink) even in GIT_SSH_COMMAND Johannes Schindelin
2017-02-01 12:01     ` [PATCH v3 1/4] connect: handle putty/plink also " Johannes Schindelin
2017-02-01 12:01     ` [PATCH v3 2/4] connect: rename tortoiseplink and putty variables Johannes Schindelin
2017-02-01 12:01     ` [PATCH v3 3/4] git_connect(): factor out SSH variant handling Johannes Schindelin
2017-02-01 12:01     ` [PATCH v3 4/4] connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config Johannes Schindelin
2017-02-01 19:19       ` Junio C Hamano
2017-02-01 19:46         ` Junio C Hamano
2017-02-01 22:24           ` Johannes Schindelin
2017-02-01 22:33             ` Junio C Hamano
2017-02-01 22:42               ` Johannes Schindelin
2017-02-01 22:43               ` Junio C Hamano
2017-02-01 23:07                 ` Johannes Schindelin
2017-02-01 20:07     ` [PATCH v3 0/4] Handle PuTTY (plink/tortoiseplink) even in GIT_SSH_COMMAND Junio C Hamano
2017-02-01 22:17       ` Johannes Schindelin
2017-02-01 22:55         ` Junio C Hamano [this message]

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqr33hmshc.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).