Skip to content

Code Review Practices

Julia Nguyen edited this page Jul 22, 2018 · 23 revisions

As mentioned in our Pull Request Practices, everyone is encouraged to participate in code reviews regardless of experience level. Code reviews are not only helpful for the person's code being reviewed. It's a great learning experience for the reviewer themselves!

Check out our Learning Resources to read up on collaboration best practices and add your recommend resources.

Our Code Review Commandments

Collaboration

  • The Contributor Convenant applies every communication channel we have in our contributor community, code reviews are no exception.
  • Code reviews are a learning experience for all parties involved so be cognizant of that. It's not an opportunity to shame others for knowing something or implementing something a certain way.
  • A lot of back and forth conversations in GitHub can create tension, so we recommend taking the conversation to another place if it's difficult to get your point across or understand something. Examples: one-on-one conversations on Slack or a voice/video call on Google Hangouts.
  • Ask lots of questions as both reviewers and reviewees! Making unconfirmed or false assumptions often leads to the negative repercussions of unconscious bias and conscious bias.
  • Be supportive! Kind gestures like words of encouragement goes a long way.
  • Please remember that you are not the code you write. No one has the answers to everything and writes code perfectly.

Code Quality

  • Code must adhere to our Frontend Practices and Backend Practices
  • Test coverage is required and must follow our Automated Testing practices.
  • We set up CircleCI, a continuous integration and delivery tool, to automatically run our tests and Codeclimate, a tool to run automated code quality checks, in pull requests. To save time, a reviewer does not have to point out syntax errors as the reviewee should update their code to meet Codeclimate's expectations. If a reviewee forgets to update their code based on Codeclimate's feedback before merging, a reviewer should kindly remind them to.
  • When describing a way to solve a coding problem, be concise and clear. You may link to articles that could help emphasize your point but don't overdo it. No one wants to be just told to read the manual.

Review Cycles

  • Please use GitHub's nifty tools on requesting reviewers. It's there for a reason!
  • Reviewers should be explicit about whether they approve a review or require changes to be made.
  • After the reviewee has made changes after the first round of reviews, the reviewee can re-add someone as the reviewer and that will trigger a notification for them to review again.
  • The Pull Requests page on GitHub is a nifty tool of keeping track of what issues are assigned to you and what PRs you were requested to review.
  • If a reviewer hasn't reviewed your code after a certain period of time you need it to be reviewed, kindly @ them in a comment in the PR with a reminder.
  • Oh no! You haven't heard back from your reviewer in a while. Unfortunately and fortunately, that's the nature of open source software. We're all working on this project voluntarily and it's important to create boundaries. As a mental health project, we gotta practice what we preach! Don't harass the reviewer to review your code. Post in #dev on Slack that you need a new pair of eyes to review.
Clone this wiki locally