Skip to content

Instantly share code, notes, and snippets.

@pistonsky
Last active May 11, 2018 08:22
Show Gist options
  • Save pistonsky/b37d9eb497a31efa64caa47f66ab533d to your computer and use it in GitHub Desktop.
Save pistonsky/b37d9eb497a31efa64caa47f66ab533d to your computer and use it in GitHub Desktop.
How to code review

How to code review?

  • Review is a top-priority task. Review code soon as possible. It can help to avoid massive merge of PRs at the end of sprint and won’t waste time of team members, especially if tasks are related to each other.
  • First of all you should check if the changes are related to the task. There shouldn’t be a thing like “let me also add this stuff to this PR by the way”.
  • Initial PR comment should contain the link to corresponding trello card.
  • You should not only check the code, but also check if the modified component is working correctly in the app. You check the code very carefully, every line. When checking the component in the app, you should test all possible use cases.
  • The code should follow established code style and styled with prettier. Also, please follow English language rules and use common sense when naming variables. Do not repeat yourself, but keep it simple.
  • Code review is not for critics, it’s rather a place for common effort to make a better product. That’s why it is important to always point to a specific mistake, so that person understands it, and also to suggest solutions for difficult situations. Do not argue with the author for too long, ask other team members about it.
  • Review all changes at once, do not review single commits one by one.
  • Code style changes and documentation changes are only allowed for places strictly related to the task. If those changes are not related to the task - open a separate PR for them. Such unrelated changes make reviews harder and potentially create merge conflicts.
  • If you make a helper function and use it in more than 1 place, put it in “utils” directory.
  • Remember: some components are interdependent, so you should also check their work.
  • If you remove some functionality, you should make sure that everything exclusively related to that functionality is removed: any utils, any common styles, metrics, colors or documentation.
  • When modifying the list of npm dependencies - check that every component that depends on that package works and that npm package versions are strictly fixed.
  • If anything is unclear in the code and you ask the author about it - ask him to add a comment about it, ask him to explain why did he choose that solution, and not the other.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment