52
52
53
53
< div id ="entry_block ">
54
54
< div class ="entry ">
55
- < div class ="date "> Last updated: 11 June 2020</ div >
55
+ < div class ="date "> Last updated: 6 October 2020</ div >
56
56
< div class ="body ">
57
57
< article >
58
58
< div class ="title "> On Reviewing, and Helping Those Who Do It</ div >
@@ -74,9 +74,9 @@ <h3 id="the-current-state-of-affairs">The Current State of Affairs</h3>
74
74
target ="_blank "> Several hundred pull requests</ a >
75
75
are open at any given moment in Bitcoin Core waiting for review
76
76
and testing, and the stack continues to grow higher — 280 in
77
- November 2019, 380 at the end of April 2020, and now 400 at the
78
- start of June, when the number of PRs in the blockers list also
79
- reached a
77
+ November 2019, 380 at the end of April 2020, and 400 at the
78
+ start of June 2020 , when the number of PRs in the blockers list
79
+ also reached a
80
80
< a href ="http://www.erisian.com.au/bitcoin-core-dev/log-2020-06-04.html#l-329 "
81
81
target ="_blank "> record high number</ a > .
82
82
</ p >
@@ -132,15 +132,10 @@ <h3 id="current-issues">Current Issues</h3>
132
132
Any PR of non-immediate urgency beyond a certain size,
133
133
number of commits, or level of complexity is a candidate for
134
134
needing to be separated into bite-sized PRs in order to
135
- attract reviewers, which multiplies the number of PRs.< br >
136
- < br >
137
- While small focused PRs are good, and separating large PRs
138
- into logical small ones can improve the code, some changes
139
- are better done in one go.
140
- </ li >
141
- < li >
142
- Grease (review, merges) sometimes goes to the squeaky
143
- wheel. Such is life.
135
+ attract reviewers, which multiplies the number of PRs.
136
+ While small focused PRs can be good, and separating large
137
+ PRs into logical small ones can improve the code, some
138
+ changes are better done in one go.
144
139
</ li >
145
140
< li >
146
141
PR authors may be incentivized to open more trivial PRs that
@@ -153,29 +148,29 @@ <h3 id="current-issues">Current Issues</h3>
153
148
Things are often abandoned or never quite finished. Some
154
149
contributors leave or take long periods of absence from the
155
150
project out of frustration. Other people shake it off but
156
- feel recurring discouragement. Yes, it's open source. But I
157
- think we can improve the situation.
151
+ feel recurring discouragement. Yes, it's open source, but I
152
+ hope we can improve the situation.
158
153
</ li >
159
154
< li >
160
155
Managing context, relevance, and being kind can make a big
161
156
difference. Bitcoin Core code review is a social
162
- process. Getting your PRs reviewed without being too
163
- annoying (hopefully by helping others and doing consistent
164
- review yourself) can be an art.
157
+ process. Having PRs reviewed without being too annoying
158
+ (hopefully by helping others and doing consistent review
159
+ yourself) can be an art.
165
160
</ li >
166
161
< li >
167
162
Nevertheless, the project and its reviewers run a continual
168
163
risk of mal-investment — scarce review resources going to
169
- poorly done, annoying, insufficiently researched, or
164
+ poorly done, annoying, insufficiently researched, or even
170
165
dangerous changes, or ones that add technical debt — rather
171
166
than catching a regression or vulnerability, fixing issues,
172
167
improving test coverage, finishing ongoing work, doing
173
168
long-term work, and other priorities.
174
169
</ li >
175
170
< li >
176
- Above all, not enough contributors do consistent review and
177
- follow the rule of thumb of 5 to 15 reviews or issues
178
- handled per PR opened.
171
+ Above all, not enough contributors put the project first, do
172
+ consistent review, and follow the rule of thumb of 5 to 15
173
+ reviews or issues handled per PR opened.
179
174
</ li >
180
175
</ ol >
181
176
</ p >
@@ -216,17 +211,15 @@ <h3 id="what-to-review">What to Review</h3>
216
211
</ li >
217
212
< li >
218
213
Who: rewarding people who do consistent or outstanding
219
- review/issue testing, or who have been kind or patient. Some
220
- of the most deserving people haven't seen enough review from
221
- me yet, but I've noticed who you are and am working on it.
214
+ review/issue testing or who have been kind and selfless.
222
215
</ li >
223
216
< li >
224
217
I also try to avoid reviewing or encouraging these kinds of
225
218
PRs:
226
219
< ol >
227
220
< li >
228
- PRs that are poorly done, annoying or unlikely to be
229
- merged .
221
+ PRs that are poorly done, annoying, appear self-serving,
222
+ or that look like commit inflation .
230
223
</ li >
231
224
< li >
232
225
PRs that are unfinished or lacking sufficient research.
@@ -278,8 +271,8 @@ <h3 id="on-reviewing">On Reviewing</h3>
278
271
and
279
272
< a href ="https://github.com/bitcoin/bitcoin/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc "
280
273
target ="_blank "> issues</ a >
281
- and choose ones that are somewhat accessible — ideally right at
282
- the edge of your current reach — and ramp up gradually to the
274
+ and review or test ones that are somewhat accessible — ideally
275
+ at the edge of your current reach — and ramp up gradually to the
283
276
harder and important
284
277
< a href ="https://github.com/bitcoin/bitcoin/projects/8 "
285
278
target ="_blank "> high-priority</ a >
0 commit comments