summaryrefslogtreecommitdiff
path: root/42/ff0671e1bb94518446e3adf8a3b2fcaeca22ce
blob: f7b0da4ac77b847c48695c0b701907d6d60becfa (plain)
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
Received: from sog-mx-3.v43.ch3.sourceforge.com ([172.29.43.193]
	helo=mx.sourceforge.net)
	by sfs-ml-2.v29.ch3.sourceforge.com with esmtp (Exim 4.76)
	(envelope-from <mh.in.england@gmail.com>) id 1VS2eE-0005kh-VH
	for bitcoin-development@lists.sourceforge.net;
	Fri, 04 Oct 2013 10:30:15 +0000
Received-SPF: pass (sog-mx-3.v43.ch3.sourceforge.com: domain of gmail.com
	designates 209.85.214.54 as permitted sender)
	client-ip=209.85.214.54; envelope-from=mh.in.england@gmail.com;
	helo=mail-bk0-f54.google.com; 
Received: from mail-bk0-f54.google.com ([209.85.214.54])
	by sog-mx-3.v43.ch3.sourceforge.com with esmtps (TLSv1:RC4-SHA:128)
	(Exim 4.76) id 1VS2eD-0002wU-OM
	for bitcoin-development@lists.sourceforge.net;
	Fri, 04 Oct 2013 10:30:14 +0000
Received: by mail-bk0-f54.google.com with SMTP id mz12so1467424bkb.13
	for <bitcoin-development@lists.sourceforge.net>;
	Fri, 04 Oct 2013 03:30:07 -0700 (PDT)
MIME-Version: 1.0
X-Received: by 10.204.123.199 with SMTP id q7mr12493863bkr.10.1380882607199;
	Fri, 04 Oct 2013 03:30:07 -0700 (PDT)
Sender: mh.in.england@gmail.com
Received: by 10.204.237.74 with HTTP; Fri, 4 Oct 2013 03:30:07 -0700 (PDT)
Date: Fri, 4 Oct 2013 12:30:07 +0200
X-Google-Sender-Auth: jac2KpG7gEkAO4YbCY4ai_CEKSg
Message-ID: <CANEZrP1Sd8cK2YUr4OSvnOxEJrbWpmfdpor-qbap1f98tGqPwg@mail.gmail.com>
From: Mike Hearn <mike@plan99.net>
To: Bitcoin Dev <bitcoin-development@lists.sourceforge.net>
Content-Type: multipart/alternative; boundary=001a11334a3c478b9004e7e7c903
X-Spam-Score: -0.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 FREEMAIL_FROM Sender email is commonly abused enduser mail provider
	(mh.in.england[at]gmail.com)
	-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: reviewboard.org]
	1.0 HTML_MESSAGE           BODY: HTML included in message
	0.1 DKIM_SIGNED            Message has a DKIM or DK signature,
	not necessarily valid
	-0.1 DKIM_VALID Message has at least one valid DKIM or DK signature
X-Headers-End: 1VS2eD-0002wU-OM
Subject: [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 10:30:15 -0000

--001a11334a3c478b9004e7e7c903
Content-Type: text/plain; charset=UTF-8

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.

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 separated.
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 introduced
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).

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.

So, I'd like to suggest the idea of using Review Board:

http://www.reviewboard.org/

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. Also
more basic ones, like side by side diffs. Branches can be and often are
submitted to the system as single reviews.

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.

--001a11334a3c478b9004e7e7c903
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Git makes it easy to fork peoples work off and create long=
 series of commits that achieve some useful goal. That&#39;s great for many=
 things. Unfortunately, code review is not one of those things.<div><br></d=
iv>
<div>I&#39;d like to make a small request - when submitting large, complex =
pieces of work for review, please either submit it as one giant squashed ch=
ange, or be an absolute fascist about keeping commits logically clean and s=
eparated. It really sucks to review things in sequence and then discover th=
at 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 introduced in ear=
lier 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).</div>
<div><br></div><div>On a related note, github seems to have lost the plot w=
ith regards to code review - they are spending their time adding 3D rendere=
rs to their diff viewer but not making basic improvements other tools had f=
or years.</div>
<div><br></div><div>So, I&#39;d like to suggest the idea of using Review Bo=
ard:</div><div><br></div><div><a href=3D"http://www.reviewboard.org/">http:=
//www.reviewboard.org/</a><br></div><div><br></div><div>It&#39;s an open so=
urce, dedicated code review tool used by lots of big name companies for the=
ir internal work. It has git[hub] integration and a lot of very neat featur=
es, like the ability to attach screenshots to reviews. Also more basic ones=
, like side by side diffs. Branches can be and often are submitted to the s=
ystem as single reviews.</div>
<div><br></div><div>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.</div><div><br></div><div><br></div></div>

--001a11334a3c478b9004e7e7c903--