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
|
# Code Review Guide
## Things to do before you start reviewing the PR
* Make sure you are familiar with the packages the PR modifies.
* Make sure you have enough continuous time to review the PR, use 300 LOC per hour to estimate.
* Make sure you can follow the updates of the PR in the next few work days.
* Read the description of the PR, if it's not easy to understand, ask the coder to improve it.
* For a bug fix PR, if there is no test case, ask the coder to add tests.
* For a performance PR, if no benchmark result is provided, ask the coder to add a benchmark result.
## Things to check during the review process
* Am I able to understand the purpose of each unit test?
* Do unit tests actually test that the code is performing the intended functionality?
* Do unit tests cover all the important code blocks and specially handled errors?
* Could procedure tests be rewritten to table driven tests?
* Is the code written following the style guide?
* Is the same code duplicated more than twice?
* Do comments exist and describe the intent of the code?
* Are hacks, workarounds and temporary fixes commented?
* Does this function do more than the name suggests?
* Can this function's behavior be inferred by its name?
* Do tests exist and are they comprehensive?
* Do unit tests cover all the important code branches?
* Could the test code be extracted into a table-driven test?
## Things to keep in mind when you are writing a review comment
* Be kind to the coder, not to the code.
* Ask questions rather than make statements.
* Treat people who know less than you with respect, deference, and patience.
* Remember to praise when the code quality exceeds your expectation.
* It isn't necessarily wrong if the coder's solution is different than yours.
* Refer to the code style document when necessary.
## Things to remember after you submitted the review comment
* Checkout Github notification regularly to keep track of the updates of the PR.
* When the PR has been updated, start another round of review or give it a LGTM.
|
