-
Notifications
You must be signed in to change notification settings - Fork 1
/
codereview-guidelines.qmd
71 lines (63 loc) · 3.76 KB
/
codereview-guidelines.qmd
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
# Guidelines {#sec-review-guidelines}
Adapted from:
- ThoughtBot's Playbook section on [code review](https://github.com/thoughtbot/guides/tree/main/code-review) under a [CC Attribution License](https://creativecommons.org/licenses/by/3.0/)
- R-Ladies Seattle presentations by [Carrie Bennett](https://drive.google.com/file/d/1rvS7nvlQnVwHfvLFVogTVY2OQ0QFCRcV/view) and [Lauren Wolfe](https://drive.google.com/file/d/1vdVCNpPWutKmoXfHbhqV_5s3UmCAPGil/view)
- SE Radio 400: [Michaela Greiler on Code Reviews](https://se-radio.net/2020/02/episode-400-michaela-greiler-on-code-reviews/)
## Everyone
- Accept that many programming decisions are opinions
- Discuss tradeoffs, which you prefer, and reach a resolution quickly.
- Offer positive comments
- What did you learn? What did you think was neat?
- Ask good questions; don't make demands
- "What do you think about naming this function `fetch_user`?"
- Good questions avoid judgment and avoid assumptions about the author's
perspective
- Ask for clarification
- "I didn't understand. Can you clarify?"
- Avoid selective ownership of code
- "Mine", "not mine", "yours"
- Avoid using terms that could be seen as referring to personal traits
- "Dumb", "stupid".
- Assume everyone is intelligent and well-meaning.
- Be explicit
- Remember people don't always understand your intentions online.
- Be humble
- "I'm not sure - let's look it up."
- Don't use hyperbole
- "Always", "never", "endlessly", "nothing"
- Don't use sarcasm
- Talk synchronously if there are too many "I didn't understand" or "Alternative solution:" comments
- Chat, screen-sharing, in person
- Post a follow-up comment summarizing the discussion.
- If you learned something new, share your appreciation
- "I did not know about this. Thank you for sharing it."
## Having Your Code Reviewed
- Be grateful for the reviewer's suggestions
- "Good call. I'll make that change."
- Be aware that it can be challenging to convey emotion and intention online
- You may want to consider using labels to convey intention and tone.
- Explain why the code exists
- "It's like that because of these reasons. Would it be more clear if I rename this class/file/method/variable?"
- Extract some changes and refactoring into future tickets/stories
- When making visual changes, include screenshots or screencasts to show the effect of the changes
- You may want to consider before/after screenshots or screencasts whenever applicable.
- Seek to understand the reviewer's perspective
- View each comment as an opportunity to learn
- If you're someone who says "sorry" frequently, think about removing this from your language
- You can ask for clarification
- It's OK to disagree. It's helpful to first look for things that you agree with and explain why you disagree
## Reviewing Code
- Communicate which ideas you feel strongly about and those you don't
- Identify ways to simplify the code while still solving the problem
- If discussions turn too philosophical or academic, move the discussion offline
- In the meantime, let the author make the final decision on alternative implementations.
- Offer alternative implementations
- But assume the author already considered them.
- "What do you think about using a custom validator here?"
- Seek to understand the author's perspective
- Remember that you are here to provide feedback, not to be a gatekeeper
- When suggesting changes using the "Add a suggestion" feature (look for this icon: {{< iconify octicon file-diff-24 >}}):
- Communicate clearly which lines you suggest adding/removing
- Test the suggested changes to validate it works whenever possible
- When not possible, let the pull request author know that you did not test the suggestion
- Provide some context to let the author know why you're suggesting the change