PR Code Review Process - SeedCompany/cord-docs GitHub Wiki

Performing a Code Review

The easiest way to perform a code review is to take a look at things one commit at a time. GitHub makes this easy; simply navigate to the Commits Tab:

image

Click on the first commit to get started. There's two different ways to leave a comment. You can leave a single comment or add a review and attach many comments to the review. You can click on the + in the gutter by the line (or lines by clicking and dragging over multiple) to leave a comment.

image

After adding the comment you can click on either "Add single comment" or "Start a review". Typically you'd only want to add a single comment if you actually only had one comment to make...otherwise a review is preferable. Note that when you start a review it is per PR, not per single commit. So once you start a review you can finish reviewing the first commit, go back to the commit list to the next commit, add more comments, etc. until you work your way through all the commits.

After you add all of your comments you can click on "Review changes" on the top right.

image

At that point you can leave a broad comment as well if you want. Sometimes you might want to do that if you don’t have any comments at all in the code and just want to basically give a thumbs up that you looked at it, or have something else more broadly to say.

Before clicking on "Submit Review" you would pick one of the three options:

  1. Comment - Just a simple way to give some feedback to the author
  2. Approve - If you've actually been tagged to do a code review and think things look good
  3. Request Changes - If you've marked something in one of your comments that you really think needs to be changed before continuing

Note that there are times when a Senior Developer will tag Entry or Mid devs to perform a code review. In this case the Senior is likely just wanting others to be aware of the changes that were made, and thus comments in the code review become more of an opportunity for the less experienced Dev to ask the Senior Developer questions. The end response of the reviewer would then just be a "Comment" or "Approve" choice upon review submission.

Receiving a Code Review

The goal here is to sharpen each other and become together devs together; don't fret if there are a lot of comments! More specifically, the goal of the reviewer is to make sure that the final result adheres to the standards we have in our codebases. The end result should be a consistent style of coding across all developers that makes for a fluid and readable codebase.

If you receive a suggestion you can make the changes in a subsequent commit and then mark the comment as resolved. There's no need to comment or react to every suggestion unless you have a specific question or comment about it. Once all the comments are resolved with either an explanation of why it doesn't need changed, or an actual commit to fix the issue, then you can request another review on the codebase. Sometimes multiple rounds of reviews are necessary...and that's okay!

After the reviewer(s) has given a thumbs up on the entire PR via an approval, then it is the developer's responsibility to actually click on the "Merge Pull Request" button so that the code is merged into the develop branch.