Skip to content

Latest commit

 

History

History
72 lines (55 loc) · 4.3 KB

code-review.md

File metadata and controls

72 lines (55 loc) · 4.3 KB

Code Review

If there are n developers in the room, then there are n+1 opinions on how things should be done.

https://google.github.io/eng-practices/review/reviewer/
https://testing.googleblog.com/2019/11/code-health-respectful-reviews-useful.html

Communication

  • 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.
  • Be humble. ("I'm not sure - let's look it up.")
  • Don't use hyperbole. ("always", "never", "endlessly", "nothing")
  • Don't use sarcasm.

Having Your Code Reviewed

  • Be grateful for the reviewer's suggestions. ("Good call. I'll make that change.")
  • Be aware of how hard it is to convey emotion and how easy it is to misinterpret feedback. If feedback seems aggressive or angry or otherwise personal, consider if it is intended to be read that way and ask the person for clarification of intent, in person if possible.
  • Assume the best intention from the reviewer's comments.
  • 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?")
  • Seek to understand the reviewer's perspective.

Reviewing Other Developers 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 to another time.
  • 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.

Review Resources

PHP

JS

  • coming soon

The Process

We use GitHub and pull requests to review code. Generally you can only create a pull request if you have another branch to compare against. With large project launches we often want to do code review over a large span of changes in the master branch. When there is no other branch to compare against we need to make one so a PR can be submitted.

You can set up a PR for the changes between any 2 commits in the commit history on a single branch like this:

  1. Start with the branch which has the commits you wish to review, for example 'master'.
  2. Make a new branch based on this branch, for example: 'master-project-z-code-review'.
  3. On this new branch, 'soft' reset back to the commit you wish to compare against the current state. This will leave all the changes between your 2 branches in your working tree.
  4. Discard the working changes for any files or updates you do want to include in the code review.
  5. If this code review is only for the back end team for example you can discard all the changes for the PHP files. Since you discarded the changes for your selected files, those will be the only changes between your branches.
  6. Commit your working tree. Push the branch.
  7. Create a new PR on github. Set the 'base' branch as your new reset branch 'master-project-z-code-review'. Set the compare branch as your original final state branch 'master'.
  8. PR can now be used for code review.