1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
|
Received: from sog-mx-1.v43.ch3.sourceforge.com ([172.29.43.191]
helo=mx.sourceforge.net)
by sfs-ml-4.v29.ch3.sourceforge.com with esmtp (Exim 4.76)
(envelope-from <pete@petertodd.org>) id 1VS3wb-000665-RS
for bitcoin-development@lists.sourceforge.net;
Fri, 04 Oct 2013 11:53:17 +0000
Received-SPF: pass (sog-mx-1.v43.ch3.sourceforge.com: domain of petertodd.org
designates 62.13.148.108 as permitted sender)
client-ip=62.13.148.108; envelope-from=pete@petertodd.org;
helo=outmail148108.authsmtp.net;
Received: from outmail148108.authsmtp.net ([62.13.148.108])
by sog-mx-1.v43.ch3.sourceforge.com with esmtp (Exim 4.76)
id 1VS3wX-0006Fz-BN for bitcoin-development@lists.sourceforge.net;
Fri, 04 Oct 2013 11:53:17 +0000
Received: from mail-c237.authsmtp.com (mail-c237.authsmtp.com [62.13.128.237])
by punt5.authsmtp.com (8.14.2/8.14.2) with ESMTP id r94Br5cr013295;
Fri, 4 Oct 2013 12:53:05 +0100 (BST)
Received: from savin (76-10-178-109.dsl.teksavvy.com [76.10.178.109])
(authenticated bits=128)
by mail.authsmtp.com (8.14.2/8.14.2/) with ESMTP id r94Br14T058995
(version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO);
Fri, 4 Oct 2013 12:53:04 +0100 (BST)
Date: Fri, 4 Oct 2013 07:53:00 -0400
From: Peter Todd <pete@petertodd.org>
To: Mike Hearn <mike@plan99.net>
Message-ID: <20131004115300.GA22215@savin>
References: <CANEZrP1Sd8cK2YUr4OSvnOxEJrbWpmfdpor-qbap1f98tGqPwg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
protocol="application/pgp-signature"; boundary="n8g4imXOkfNTN/H1"
Content-Disposition: inline
In-Reply-To: <CANEZrP1Sd8cK2YUr4OSvnOxEJrbWpmfdpor-qbap1f98tGqPwg@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
X-Server-Quench: 8744d1c1-2ceb-11e3-94fa-002590a135d3
X-AuthReport-Spam: If SPAM / abuse - report it at:
http://www.authsmtp.com/abuse
X-AuthRoute: OCd2Yg0TA1ZNQRgX IjsJECJaVQIpKltL GxAVKBZePFsRUQkR
aAdMdgYUF1YAAgsB AmUbWlZeVV57WWc7 bAxPbAVDY01GQQRq
WVdMSlVNFUsqCBhz RB8aNhl3fwxPfTBx Z0BhXj5TW0B+IUcu
Q1NWRjhUeGZhPWMC WUQOJh5UcAFPdx8U a1N6AHBDAzANdhES
HhM4ODE3eDlSNilR RRkIIFQOdA4gGTIx DwoPAzQiAiUA
X-Authentic-SMTP: 61633532353630.1024:706
X-AuthFastPath: 0 (Was 255)
X-AuthSMTP-Origin: 76.10.178.109/587
X-AuthVirus-Status: No virus detected - but ensure you scan with your own
anti-virus system.
X-Spam-Score: -1.5 (-)
X-Spam-Report: Spam Filtering performed by mx.sourceforge.net.
See http://spamassassin.org/tag/ for more details.
-1.5 SPF_CHECK_PASS SPF reports sender host as permitted sender for
sender-domain
-0.0 SPF_PASS SPF: sender matches SPF record
0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked.
See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block
for more information. [URIs: petertodd.org]
X-Headers-End: 1VS3wX-0006Fz-BN
Cc: Bitcoin Dev <bitcoin-development@lists.sourceforge.net>
Subject: Re: [Bitcoin-development] Code review
X-BeenThere: bitcoin-development@lists.sourceforge.net
X-Mailman-Version: 2.1.9
Precedence: list
List-Id: <bitcoin-development.lists.sourceforge.net>
List-Unsubscribe: <https://lists.sourceforge.net/lists/listinfo/bitcoin-development>,
<mailto:bitcoin-development-request@lists.sourceforge.net?subject=unsubscribe>
List-Archive: <http://sourceforge.net/mailarchive/forum.php?forum_name=bitcoin-development>
List-Post: <mailto:bitcoin-development@lists.sourceforge.net>
List-Help: <mailto:bitcoin-development-request@lists.sourceforge.net?subject=help>
List-Subscribe: <https://lists.sourceforge.net/lists/listinfo/bitcoin-development>,
<mailto:bitcoin-development-request@lists.sourceforge.net?subject=subscribe>
X-List-Received-Date: Fri, 04 Oct 2013 11:53:18 -0000
--n8g4imXOkfNTN/H1
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Fri, Oct 04, 2013 at 12:30:07PM +0200, Mike Hearn wrote:
> Git makes it easy to fork peoples work off and create long series of
> commits that achieve some useful goal. That's great for many things.
> Unfortunately, code review is not one of those things.
>=20
> I'd like to make a small request - when submitting large, complex pieces =
of
> work for review, please either submit it as one giant squashed change, or
> be an absolute fascist about keeping commits logically clean and separate=
d.
> It really sucks to review things in sequence and then discover that some
> code you spent some time thinking about or puzzling out got
> deleted/rewritten/changed in a later commit. It also can make it harder to
> review things when later code uses new APIs or behaviour changes introduc=
ed
> in earlier commits - you have to either keep it all in your head, do lots
> of tab switching, or do a squash yourself (in which case every reviewer
> would have to manually do that).
When I'm reviewing multiple commit pull-requests and want to see every
change made, I always either click on the "Files Changed" tab on github,
which collapses every commit into a single diff, or do the equivalent
with git log.
Why doesn't that work for you?
> On a related note, github seems to have lost the plot with regards to code
> review - they are spending their time adding 3D renderers to their diff
> viewer but not making basic improvements other tools had for years.
>=20
> So, I'd like to suggest the idea of using Review Board:
>=20
> http://www.reviewboard.org/
>=20
> It's an open source, dedicated code review tool used by lots of big name
> companies for their internal work. It has git[hub] integration and a lot =
of
> very neat features, like the ability to attach screenshots to reviews. Al=
so
> more basic ones, like side by side diffs. Branches can be and often are
> submitted to the system as single reviews.
>=20
> The company behind it (disclosure - written and run by a long time friend
> of mine) offers hosting plans, but we could also host it on a Foundation
> server instead.
One advantage of using github is that they're an independent third
party; we should think carefully about the risks of furthering the
impression that Bitcoin development is a closed process by moving the
code review it to a server that we control with explicit review groups.
Given that Review Board appears to remain cryptographically unverifiable
there may also be disadvantages in operating it ourselves in that if the
review server does get compromised we *don't* have a third-party to
blame. In addition GitHub is a third-party with a very valuable
reputation to uphold and full-time staff - they're doing a better job of
keeping their servers secure and running then we ever could.
--=20
'peter'[:-1]@petertodd.org
--n8g4imXOkfNTN/H1
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
iQEcBAEBCAAGBQJSTqwcAAoJECSBQD2l8JH71bwH/Ap1Mkguf27+ycuDW4yLLj23
DoQrbLGYDFvBLeXVmIYZ0+goIlNQPpf+B/II+i+lQOd7RPeNz+WjQLsdMiwbKjGT
wVGXM6KuE08JxhpR0HphTVkJm5UxHc4h+Y7bprXsAYBR6PTEoc20sN53q99G0lHF
i/H/3n+AaRdnjRuYCwbI5zHSMfje4TYUflocQ9yNWF7bdYMSvGYANJGxnRk2/2F2
nB37PT89Kx+DQ3okbusmwQm1nOfzfrh1jg3St4DHYL5qc2oiGl+w0ND9HymW4YKE
VEPNP6sn8w0UMOl6T1ZFRYCDsZntKifhLl4pGLpwKN8jtiHZi+vicEH9oGqWuQs=
=ZRfu
-----END PGP SIGNATURE-----
--n8g4imXOkfNTN/H1--
|
