The Travis checks on my pull request succeeded and now it's ready for review.

Reviews should be a positive experience, regardless of the outcome. All feedback should be constructive and positive. Keep in mind, all feedback provided is regarding your code, not you as a contributor. Everyone involved needs to be receptive to feedback and willing to participate in the conversation.

Remember to be patient. Sometimes it can take a bit for someone to review your code. But, don't worry, someone will get to it. Your contributions are a huge part of what makes CircuitPython amazing! Providing feedback to help grow our community contributions is incredibly important to us.

I've waited a bit, and I received an email saying there was an update to my PR. Time to take a look!

Carter, @caternuson on GitHub, has reviewed my PR! Thanks, Carter! 

Some PRs are ready to go on the first try, and will be merged immediately. If this happens, excellent! You can skip to the end of this section to find out how to continue.

However, this may not always happen. Don't be discouraged if someone requests changes on your PR. It happens all the time. There are many reasons to request changes. The reviewer may have a different perspective on things than you do and suggest a way to do things that you might not have considered, or you may not be aware of a particular format or standard that we follow. It's all part of the review process.

Carter has requested some changes on my PR. The status at the bottom alters to reflect the Changes requested status.

As well, there's a red X next to his name under Reviewers in the right column, a red circle with an X in it next to his name above the change request. A change request is exactly what it sounds like, a request for changes to the code you've submitted for review.

The change request is identifiable because above it, there's a line above it stating caternuson requested changes 42 seconds ago. Let's take a look at the requested changes.

With a more involved multi-section review, it's a good idea to click the blue View Changes button found on the right side above the change request. This will take you to the Files Changed tab, which would now include a line-by-line review. Since there's only one section to the review, I'm going to view it here.

Carter pointed out that my method for dealing with the stop_tone / play_file issue could be done in a simpler way. As well, he suggests using a different method to deal with the play_file bug I found. These are both great suggestions! I hadn't considered doing it that way, but it definitely makes more sense than the way I did it. So I'm going to incorporate Carter's suggested changes.

Discussing the Review

There will come a time when you receive a suggestion in a review that doesn't make sense or you don't agree with. You have every right to ask questions or discuss any part of a review. You can reply by clicking in the Reply box and typing your response. Pull requests are setup to handle forum-like discussions. Feel free to ask for clarification, explain the reason you chose to do something, simply thank someone for their assistance, or open any form of discussion you feel is needed for your review. Some more involved PRs have extremely lengthy discussions as code goes through multiple iterations and changes. This is great! You should always feel comfortable continuing the discussion if you feel it's necessary. We definitely do!

Submitting the Requested Changes

Since I agree with Carter's suggestions, I'm going to make the changes. I will again follow the same steps I did to submit the correction when Travis failed (just as I did in Status, Status, Commit, Push). I begin by adding the changes into my code. I then test it. It works successfully. Great! Time to check the status and diff.

Those are the changes Carter suggested. Now it's time to check status, add, status, commit, status, push, the same as I did when I was submitting linting fixes.

Pushing the current branch will automatically update the open PR following a change request, exactly as it did during the code checking phase. This is true for the entire duration of an open pull request, regardless of the reason for submitting updates.

Now that my changes are submitted, it's time to return to GitHub to view the updated PR.

This will trigger a new Travis build. It passed!

Since I addressed all of the requested changes, the change request is now collapsed with the option to Show outdated. This is followed by my commit, which has the green check showing it passed the checks. It's always good practice to include a comment to let the reviewer know what you did to address their changes. I've commented to let Carter know that I made the changes and tested them successfully.

The status at the bottom will remain red until Carter approves the changes.

If I click on the Files Changed tab, it will reflect the new changes from the most recent commit. You'll see that the green and red numbers and boxes at the top have changed to reflect the most recent commit as well.

Now I'll wait until Carter has the opportunity to take a look at the updates I made. When he does, I will receive an email letting me know there's been an update to my pull request.

Pull Request Approved

I received the email letting me know there was an update to my pull request. It's been approved! Carter has reviewed the changes I made following his change request and concluded that I've made them to his satisfaction. He comments to let me know this is the case, and approves the changes.

The only thing left is for Carter to merge my changes into the original project. This often happens in quick succession following the approval, and you may never see the status above.

Once the merge is completed, the status changes to Pull request successfully merged and closed. The status icons that indicate a successfully merged PR are purple.

The status at the top of the page also changes to purple to reflect Merged status. You'll see that it now outlines how many commits were merged, as well as the branches involved.

That's the end of the pull request process. Congratulations! Your code is now merged with the original project and available for everyone to use.

Now it's time to update your own master branch with your new code. The next section shows you how to update your local repo and your fork on GitHub.

This guide was first published on Jun 29, 2018. It was last updated on Jun 29, 2018.
This page (Receiving a Review) was last updated on Jul 25, 2020.