Checklist for Reviewing Your Teammates' Work

This document provides guidance for how team members to review each others' work. It is primarily aimed at reviewing code but can be adapted to other types of work, e.g., diagrams, UI mockups, etc.

Each team member is expected to review another team members' work each sprint. This review can be done in pairs, and multiple team members may review the same work.

When completing your review, fill out the checklist below.

Table of Contents

1. Checklist for Reviewing Your Teammates' Work
   1. Checklist
   2. Additional Guidance for Reviewers

Checklist

1. What is the "definition of done" for this task?
   • What are the expected and actual deliverables? Are all expected deliverables present?
2. Does it work? This could include the following:
   • UI Design:
     • Does the user interface support the needed actions?
     • Is the relevant information displayed to the user?
     • Is the UI easy to use?
     • Does the UI meet accessibility standards?
   • Database/API/Other function, class or module interfaces:
     • Does the exposed interface take in the parameters it needs to and return the correct expected data?
     • Is it clear which inputs are allowed and disallowed?
     • Are disallowed inputs handled well?
   • Development tools, e.g., linters, test runners, formatters, and dev stack
     • Are the instructions complete? That is, can everyone on the team achieve the desired result without the person who wrote the instructions intervening?
   • Does the solution identify and handle core use cases and edge cases?
3. Does the solution meet non-functional requirements? This could include:
   • Computation time, memory use, response time, or other performance constraints
   • Following security best-practices
   • End-user-facing "unspecified" behaviors should be minimized. If the user might plausibly encounter a program state, you should specify and implement a reasonable outcome.
4. Is the work understandable?
   • Is the effort you spent to comprehend the work proportional to the complexity needed for a proper solution?
     • Note: Understanding someone else's work is typically more effortful than understanding your own work, but you should aim for your work to be understandable by everyone on the team without having you directly explain it.
   • Does the work include documentation that helps the reader understand it? E.g.:
     • Comments explaining functions/classes/lines of code that are more complex than a "middle school reading level."
     • Does the documentation point towards relevant external documentation or background information for 3rd-party library use or domain-specific knowledge?
5. Is it thoroughly tested? This includes:
   • Are there automated tests for the full range of general and edge cases?
     • If your work doesn't have thorough automated tests, you need a VERY good reason for that. "It takes too long" is not a good reason.
   • Are the choices of unit/integration/end-to-end tests appropriate?
   • Note: Understandability and code quality principles apply to tests too.
   • Are the tests organized in a way that makes it easy for the reader to verify that the tests cover the full range of behaviors?
   • When you add a regression test, add a comment to the test stating what the bug was. The comment should also reference the specific backlog item for the bug (i.e., include a link to the issue).
6. Code quality and miscellaneous
   • Do you see anything that could or would cause an issue in production?
   • Are function/class/variable names well-chosen?
   • Are commit messages informative and professional?

Additional Guidance for Reviewers

• Be mindful of the difference between "this solution/code has a specific issue" and "I would have done this differently".
• Don't "bikeshed." "Bikeshedding" is gicking on trivial things while neglecting more important ones.
• It's ok to comment on minor things, but don't make an ordeal out of it. One approach is to prefix your comment with "Nit:", i.e.:
  • Nit: Could turn lines 4-8 into a helper function
  • Nit: Snake case for consistency
  • Typo: spams -> spam