From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Eric Wong <e@80x24.org>, Stefan Beller <sbeller@google.com>,
git@vger.kernel.org, Johannes Sixt <j6t@kdbg.org>
Subject: Re: [PATCH 1/2] xread: retry after poll on EAGAIN/EWOULDBLOCK
Date: Mon, 27 Jun 2016 10:36:48 -0400 [thread overview]
Message-ID: <20160627143648.GA2618@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqqoa6mdbu3.fsf@gitster.mtv.corp.google.com>
On Mon, Jun 27, 2016 at 06:02:12AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I also wondered how we managed to miss such an obvious point in review
> > of the original patch. Sadly, we _did_ notice it[1] but it looks like we
> > never fixed the problem. That is even more disturbing.
>
> Yes indeed.
>
> I try to pay attention to "this is broken because..." comments in
> discussions to make a note in my copy of "What's cooking" report for
> a problematic topic, as that is where I work from when merging
> topics down, but apparently that procedure failed work in this case.
> There needs a stronger mechanism to stop a known-buggy patch from
> going thru, but I am not sure offhand what that should be.
I was the one who saw the bug. I could have followed the series more
closely to make sure my concern was addressed. Or possibly pointed out
the bug more prominently than an in a "PS" as part of the discussion.
I think part of the problem was that this particular series was
large-ish and involved a lot of re-rolls, and I got sick of looking at
it. I dunno.
It's also true that our error rate will never be 0%. So some bugs will
always slip through, some review comments will be forgotten, etc. Eric
did find and fix the bug just now, so the "many eyes" theory did work
here eventually.
-Peff
next prev parent reply other threads:[~2016-06-27 14:37 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-26 23:21 [PATCH 0/2] wrapper: xread/xwrite fixes for non-blocking FDs Eric Wong
2016-06-26 23:21 ` [PATCH 1/2] xread: retry after poll on EAGAIN/EWOULDBLOCK Eric Wong
2016-06-26 23:42 ` Jeff King
2016-06-27 13:02 ` Junio C Hamano
2016-06-27 14:36 ` Jeff King [this message]
2016-06-27 16:49 ` Stefan Beller
2016-06-27 19:17 ` Jeff King
2016-06-27 19:43 ` Stefan Beller
2016-06-27 19:51 ` Jeff King
2016-06-27 20:13 ` Eric Wong
2016-06-27 21:49 ` Jeff King
2016-06-27 22:22 ` Jeff King
2016-06-27 23:19 ` Eric Wong
2016-06-26 23:51 ` Jeff King
2016-06-27 3:56 ` [PATCHv2 " Eric Wong
2016-06-26 23:21 ` [PATCH 2/2] xwrite: poll on non-blocking FDs Eric Wong
2016-06-26 23:49 ` Jeff King
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=20160627143648.GA2618@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=e@80x24.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=sbeller@google.com \
/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).