Formatting of Code Review guidelines for better readability (#688)

thoughtbot · Oct 6, 2023 · 5f957c0 · 5f957c0
1 parent 27f324d
commit 5f957c0
Showing 1 changed file with 90 additions and 65 deletions.
diff --git a/code-review/README.md b/code-review/README.md
@@ -1,76 +1,101 @@
 # Code Review
 
-A guide for reviewing code and having your code reviewed. Watch a presentation
-that covers this material from [Derek Prior at RailsConf
-2015](https://www.youtube.com/watch?v=PJjmw9TRB7s).
+A guide for reviewing code and having your code reviewed.
+
+Watch a presentation that covers this material from [Derek Prior at RailsConf 2015](https://www.youtube.com/watch?v=PJjmw9TRB7s).
 
 ## Everyone
 
-- Accept that many programming decisions are opinions. Discuss tradeoffs, which
-  you prefer, and reach a resolution quickly.
-- Ask good questions; don't make demands. ("What do you think about naming this
-  `:user_id`?")
-- 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.
-- Keep it real. If emoji, animated gifs, or humor aren't you, don't force them.
-  If they are, use them with aplomb.
-- Talk synchronously (e.g. chat, screen-sharing, in person) if there are too
-  many "I didn't understand" or "Alternative solution:" comments. 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".)
+- **Accept that many programming decisions are opinions**
+  - Discuss tradeoffs, which you prefer, and reach a resolution quickly.
+
+- **Ask good questions; don't make demands**
+  - "What do you think about naming this `:user_id`?"
+
+- **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**
+- **Keep it real**
+  - If emoji, animated gifs, or humor aren't you, don't force them.
+  - If they are, use them with aplomb.
+
+- **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.
-- Link to the code review from the ticket/story. ("Ready for review:
-  https://github.com/organization/project/pull/1")
-- Push commits based on earlier rounds of feedback as isolated commits to the
-  branch. Do not squash until the branch is ready to merge. Reviewers should be
-  able to read individual updates based on their earlier feedback.
-- Seek to understand the reviewer's perspective.
-- Try to respond to every comment.
-- Wait to merge the branch until continuous integration (TDDium, Travis CI,
-  CircleCI, etc.) tells you the test suite is green in the branch.
-- Merge once you feel confident in the code and its impact on the project.
-- Final editorial control rests with the pull request author.
+- **Be grateful for the reviewer's suggestions**
+  - "Good call. I'll make that change."
 
-[challenging to convey emotion and intention online]: https://thoughtbot.com/blog/empathy-online
-[using labels]: https://conventionalcomments.org/
+- **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**
+- **Link to the code review from the ticket/story**
+  - "Ready for review: https://github.com/organization/project/pull/1"
+
+- **Push commits based on earlier rounds of feedback as isolated commits to the branch**
+  - Do not squash until the branch is ready to merge.
+  - Reviewers should be able to read individual updates based on their earlier feedback.
+
+- **Seek to understand the reviewer's perspective**
+- **Try to respond to every comment**
+- **Wait to merge the branch until continuous integration tells you the test suite is green in the branch**
+  - TDDium, Travis CI, CircleCI, Github Actions, etc.
+
+- **Merge once you feel confident in the code and its impact on the project**
+- **Final editorial control rests with the pull request author**
 
 ## Reviewing Code
 
-Understand why the change is necessary (fixes a bug, improves the user
-experience, refactors the existing code). Then:
-
-- 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
-  to a regular Friday afternoon technique discussion. 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.
-- Approve the pull request.
-- Remember that you are here to provide feedback, not to be a gatekeeper.
+Understand why the change is necessary (fixes a bug, improves the user experience, refactors the existing code).
+
+Then:
+
+- **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 to a regular Friday afternoon technique discussion**
+  - 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**
+- **Approve the pull request**
+- **Remember that you are here to provide feedback, not to be a gatekeeper**
 - When suggesting changes using the "Add a suggestion" feature:
-  - 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.
+  - **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**
 
 ## Style Comments
 
@@ -82,10 +107,10 @@ An example response to style comments:
 
     Whoops. Good catch, thanks. Fixed in a4994ec.
 
-If you disagree with a guideline, open an issue on the guides repo rather than
-debating it within the code review. In the meantime, apply the guideline.
-It's often helpful to set up a linter like [standard] to format code automatically.
-This helps us have more meaningful conversations on PRs rather than debating
-personal style preferences.
+If you disagree with a guideline, open an issue on the guides repo rather than debating it within the code review. In the meantime, apply the guideline. It's often helpful to set up a linter like [standard] to format code automatically.
+
+This helps us have more meaningful conversations on PRs rather than debating personal style preferences.
 
+[challenging to convey emotion and intention online]: https://thoughtbot.com/blog/empathy-online
+[using labels]: https://conventionalcomments.org
 [standard]: https://github.com/testdouble/standard