Review 1st Pull Request: asking for corrections
As the owner, it’s time to review the Pull Requests the rest of your team mates have opened.
We’ve actually snuck some errors into the instructions we provided in your team mates issues. The PR review stage provides opportunity for any problems to be identified and dealt with before merging.
Let’s use the first pull request made to the repo as an example. In this case, it was made by team mate bobturneruk, tackling issue #2 Add subtraction function via pull request #5.
Review 1st Subtract Pull Request
As I mentioned, there is at least one error in the Pull Request as a result of erroneous instructions in the issue.
Problems are also indicated by the failing tests which we can inspect.
Identify the errors
The failing tests, and in particular, the linting check by flake8 points to a missing colon in the subtract function definition.
You may also have additional errors if the instructions in the issue were followed incorrectly by your team mates. No problem though! The review stage will help us inspect and guide our team mates to correct any additional errors.
To check that everything has been completed correctly, refer to the Appendix in this chapter, which has the correct files and file content that should have been committed for each issue.
Review pull request and request changes
Start the review by first inspecting the changes to the files made in the pull request by clicking on the Files changed tab of the PR.
This tab shows all added and deleted lines of code in each file commited in the PR. It also allows us to post review comments to an individual line of code, by clicking on the at the left hand side margin of the line of code.
This opens a box to add your review comment. Once you’ve added your first comment, click on Start a review to add your comment and initiate a review.
In the case of the subtract PR, I identified the line of code containing the error (line #1 in pythoncalculator/subtract.py) and added a comment identifying the error for my team mate to correct.
GitHub has automatically flagged a couple more issues (2 files that don’t end in a newline, indicated by the ) so I’ve also asked my team mate to correct those.
To submit my review, I click on Finish your review in the top-right corner and select the Changes requested option.
This posts my comments into the PR discussion tab where the author of the PR can respond to them. The fact that I’ve requested changes is also flagged.
Correct errors and respond to request for changes
Now my team can go back to GitKraken Client and make the corrections to any errors I’ve flagged
Documentation on Editing files in GitKraken Client
Once complete, they can then commit the corrections in their issue branch and push again to GitHub. The new commits will be added to the PR.
Next, my team mate moved back to the PR in GitHub and responded to each error individually to let me know it had been corrected.
Complete final approving review.
Check and resolve error conversations
At this point, I went back and checked the changes to the files myself. I also ensured that all checks were passing.
If you still spot problems when you’re reviewing the changes, repeat the process we’ve just been through.
If all checks aren’t passing, have a look at the logs of the checks and/or re-inspect the changes to the files to spot any problems. Then repeat the process we’ve just been through.
Once I was happy each one had been corrected, I responded also and resolved each point individually.
We can also see the conversations have an Outdated tag associated with them. This indicates that new commits have been made since the discussion was initiated.
Initiate final approving review
Next I’ll submit a final review, this time selecting the Approve option.
Merge Pull Request
Once the approving review is submitted and given all the checks pass, the pull request is now ready to merge into the main branch! 🎉
To initiate the merge, I click on Merge pull request
Next, I click on Confirm merge
The subtract branch has now successfully been merged into main! 🥳
I’ve also gone ahead and deleted the origin subtract branch to keep the repo tidy as the branch is now redundant (i.e. I don’t expect any further contributions in this branch regarding the particular feature it was created for).
Review of the first pull request is now complete!