How to review a pull request

Reviewing other people's code is important for three reasons:

1. You get to weigh in on the design of the code, before it is officially added to the robot.
2. You may catch a bug or mistake that would have gone by unnoticed.
3. You familiarize yourself with the relevant portion of the codebase, and may even learn a thing or two!

All pull requests must be approved by at least one other person before they can be merged. You may occasionally receive a request to review a PR, but that is more of a suggestion than a requirement. In reality, you are encouraged to review PRs that you find interesting, even ones that have already been reviewed! The more eyes the better.

This guide hopes to document how to review a pull request, as well as some tips and best practices.

Viewing changed files

You can view all of the changes in a pull request by clicking the "Files changed" button.

This will show you a list of all files changed, in a format known as a diff (or difference). The green right side is text added, while the red left side is text removed.

In the above example, the text 4.2 on line 3 was removed and replaced with 6.4. Additionally, the date the file was last modified was added on line 1.

Your job is to look for improvements and issues with the changes. If a piece of code can be improved or if one of the changes doesn't look right, hover over it and click the blue plus to add a comment.

You can select "Add single comment," which will publish the comment immediately, or "Start a review," will let you publish all of your comments together in a single batch. I recommend selecting "Start a review" for most scenarios, unless you have a once-off comment.

When you have finished looking over a file, click the "Viewed" checkbox to minimize it. This is one of the best ways to keep track of your progress as you review a pull request. If the author makes additional changes after you review the pull request, any affected files will be automatically un-minimized.

When you have finished reviewing all changes, scroll back to the top of the page and select "Review changes." Here you may select whether you approve the changes, request changes, or just comment on the pull request.

1. Approve - You agree with the changes and are content with them being merged as they currently are.
2. Request changes - There is something that needs fixed. Selecting this will prevent the pull request from being merged until it is resolved.
3. Comment - You just looked over the code, though are not giving an explicit approval.

Once you are done, select "Submit review" to publish it. Thank you!