Skip to content

Commit 480b6b8

Browse files
authored
A few suggestions. I can use some language adjustment (#705)
1 parent 45eeb59 commit 480b6b8

File tree

1 file changed

+23
-7
lines changed

1 file changed

+23
-7
lines changed

code-review/README.md

+23-7
Original file line numberDiff line numberDiff line change
@@ -15,23 +15,26 @@ Watch a presentation that covers this material from [Derek Prior at RailsConf 20
1515
- **Good questions avoid judgment and avoid assumptions about the author's
1616
perspective**
1717
- **Ask for clarification**
18-
- "I didn't understand. Can you clarify?"
18+
- "I didn't understand. Can you clarify?"
1919

2020
- **Avoid selective ownership of code**
21-
- "Mine", "not mine", "yours"
21+
- "Mine", "not mine", "yours"
2222

2323
- **Avoid using terms that could be seen as referring to personal traits**
2424
- "Dumb", "stupid".
2525
- Assume everyone is intelligent and well-meaning.
2626

27+
- **Avoid diminishing words**
28+
- "simply", "simple", "just"
29+
2730
- **Be explicit**
28-
- Remember people don't always understand your intentions online.
31+
- Remember people don't always understand your intentions online.
2932

3033
- **Be humble**
31-
- "I'm not sure - let's look it up."
34+
- "I'm not sure - let's look it up."
3235

3336
- **Don't use hyperbole**
34-
- "Always", "never", "endlessly", "nothing"
37+
- "Always", "never", "endlessly", "nothing"
3538

3639
- **Don't use sarcasm**
3740
- **Keep it real**
@@ -43,8 +46,12 @@ Watch a presentation that covers this material from [Derek Prior at RailsConf 20
4346
- Post a follow-up comment summarizing the discussion.
4447

4548
- **If you learned something new, share your appreciation**
46-
- "I did not know about this. Thank you for sharing it."
49+
- "I did not know about this. Thank you for sharing it."
4750

51+
- **Avoid the "since you're at it" attitude**
52+
- If you would like to recommend a code change unrelated to the current
53+
pull request, suggest it in the appropriate place or open a ticket for it
54+
(on Trello, JIRA, GitHub project...)
4855

4956
## Having Your Code Reviewed
5057

@@ -71,11 +78,14 @@ Watch a presentation that covers this material from [Derek Prior at RailsConf 20
7178
- **Seek to understand the reviewer's perspective**
7279
- **Try to respond to every comment**
7380
- **Wait to merge the branch until continuous integration tells you the test suite is green in the branch**
74-
- TDDium, Travis CI, CircleCI, Github Actions, etc.
81+
- TDDium, Travis CI, CircleCI, GitHub Actions, etc.
7582

7683
- **Merge once you feel confident in the code and its impact on the project**
7784
- **Final editorial control rests with the pull request author**
7885

86+
- **Recognize the work of your teammates when you are pairing**
87+
- Use `Co-Authored-By: <name> <email>` at the end of your commit message.
88+
7989
## Reviewing Code
8090

8191
Understand why the change is necessary (fixes a bug, improves the user experience, refactors the existing code).
@@ -114,6 +124,12 @@ If you disagree with a guideline, open an issue on the guides repo rather than d
114124

115125
This helps us have more meaningful conversations on PRs rather than debating personal style preferences.
116126

127+
- **Leave one comment only, for multiple stylistic offenses of the same kind**
128+
- If there are a few occurrences of the same change needed, do not
129+
leave multiple comments for the same change, rather suggest running the linter,
130+
and/or leave one comment only, mentioning the line and elsewhere,
131+
as long as the other files are being edited in the pull request.
132+
117133
[challenging to convey emotion and intention online]: https://thoughtbot.com/blog/empathy-online
118134
[using labels]: https://conventionalcomments.org
119135
[standard]: https://github.com/testdouble/standard

0 commit comments

Comments
 (0)