For Reviewer ​

Functionality ​

Code reviewers must ensure the pull request (PR) passes all AC for a story. This check comes in addition to the functional testing done by the code author, i.e., the code author must not rely on code reviewers to catch basic functional issues on their behalf.

Code reviewers should focus their attention on the following:

• Edge cases or bugs that the code author might not have considered.
• Performance and security concerns.
• Putting themselves in the user’s shoes (i.e., thinking of the User Experience).

Conventions ​

While there are no magic rules, developers must ensure they pick good names for everything:

• A good name must be descriptive enough to fully communicate what the item is or does without being so long that it becomes hard to read. Very concise names without meaning, such as (‘a’, ‘i’, etc.) must be avoided.
• Naming must follow the standard names on a specific stack.

Testing ​

Testing is required during development. So code reviewers must ensure author writes tests to cover their implementation code and that all added/modified tests are well-designed.

Positive areas ​

While code reviewers tend to focus on areas of improvement, they should offer encouragement and appreciation for good implementation and practices, e.g.:

• The implementation is well described in the pull request description, making the review a breeze.
• The code author has thought of edge cases that the code reviewer did not know about.
• The code author has used practical algorithms that could improve complex logic effectively.

Speed of code review ​

Be aware of the speed When code reviews are slow, several things happen:

• The velocity of the team as a whole is decreased.
• Code health can be impacted.

When reviewers are slow, there is increased pressure on both:

• Code reviewer: give review depth/quality that is not as detailed/good as it could be.
• Author: allow the author(s) to submit PRs that are not as good as they could be.

How fast should code review be?

• Reviewers should do a code review shortly after it comes in if they are not in the middle of a focused task.
• One business day is the maximum time it should take to respond to a code review request (i.e., the first thing the next morning).
• Following these guidelines means that a typical PR should get multiple rounds of review (if needed) within a single day.

Giving comments or feedback In general, code reviewers should be courteous and respectful while being very clear and helpful to code authors of the pull request. Code reviewers must avoid making code authors upset or contentious by always commenting about the code and never commenting about authors.

Be kind. Be humble. (I’m not sure - let’s look it up.) Ask for clarification. (I didn’t understand. Can you clarify?) Avoid selective ownership of code. (mine, not mine, yours) Don’t use hyperbole. (always, never, endlessly, nothing)

Giving guidance In general, the responsibility of the author is to fix the pull request, not the code reviewer. The code reviewers are not required to give a detailed solution or write code for the author of the pull request.

• When giving guidance, the code reviewer should explain the reasoning why. If there are concerns or questions from the author, the code reviewer might consider bringing the topic to Slack for discussions or event calls if there is too much confusion. After that, post a follow-up comment summarizing the PR discussion.
• Encourage code authors to simplify the code or add code comments instead of just explaining the complexity.
• Balance giving explicit direction with just pointing out problems and letting code authors decide.
• Accept that many programming decisions are opinions. Discuss tradeoffs, which you prefer, and resolve them quickly.