-
Notifications
You must be signed in to change notification settings - Fork 0
Expand file tree
/
Copy pathgood_reviews.html
More file actions
40 lines (38 loc) · 1.51 KB
/
good_reviews.html
File metadata and controls
40 lines (38 loc) · 1.51 KB
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
<!DOCTYPE html>
<html><head><meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>Avi's thoughts on good reviews</title>
<link rel="stylesheet" href="css.css">
</head>
<body>
<a href="index.html">⬅ back to the index</a><br>
<hr>
<h1>Good Reviews</h1>
Reviews exist on a spectrum from low value to high value.
<h3>Low Value Reviews</h3>
A low value review takes on faith that the objectives and approach should not be improved. Without explanation and evidence, this is never a correct assumption.
<br><br>
A low value review stops after asking...
<ul>
<li>Can I understand this code?</li>
<li>Does this code look like it will work?</li>
</ul>
A low value reviewer says "This seems weird to me. The problem must be me."
<p>
</p>
<h3>High Value Reviews</h3>
A high value review questions every aspect of a change, including why it's being done in the first place.
<br><br>
A high value review asks...
<ul>
<li>What high level goal is this change trying to accomplish?</li>
<li>Is the objective well-defined?</li>
<li>Is the objective actually good or is it mistaken?</li>
<li>At a conceptual level, is the general approach taken very good or can it be obviously improved?</li>
<li>Does the code implement the approach well or poorly? How might the code be made to feel more elegant and informative?</li>
<li>What circumstances will the new code NOT handle?</li>
</ul>
A high value reviewer says "This seems weird to me. The problem might not be me. I should try to find out."
<br>
</body>
</html>