QuakeCoRE Software Team has introduced code review policy as an effort for quality assurance and encourage increased interactions amongst team members.
Step-by-step guide
Developer : Create a branch -> Edit -> Add/Commit/Push to the branch
In the following, let's assume the developer "seb56" is making a small change to "git_sim_pkg" repository.
seb56@hypocentre /home/seb56/gm_sim_pkg :git checkout -b fix_hardcoded_python Switched to a new branch 'fix_hardcoded_python'
https://www.atlassian.com/git/tutorials/using-branches
Suppose the developer edited the file "setup_remote.sh. The developer does "git add" followed by "git commit".seb56@hypocentre /home/seb56/gm_sim_pkg/wrapper :git add setup_remote.sh seb56@hypocentre /home/seb56/gm_sim_pkg/wrapper :git commit -m "Replaced the hard-coded python path with a variable" [fix_hardcoded_python c2a7911] Replaced the hard-coded python path with a variable 1 file changed, 2 insertions(+), 1 deletion(-)
When the developer feels the code is ready, he/she does "git push" to the new branch.
seb56@hypocentre /home/seb56/gm_sim_pkg/wrapper :git push origin fix_hardcoded_python Counting objects: 4, done. Delta compression using up to 32 threads. Compressing objects: 100% (4/4), done. Writing objects: 100% (4/4), 410 bytes | 0 bytes/s, done. Total 4 (delta 3), reused 0 (delta 0) remote: Resolving deltas: 100% (3/3), completed with 3 local objects. To github.com:ucgmsim/gm_sim_pkg.git * [new branch] fix_hardcoded_python -> fix_hardcoded_python
Developer : Make a pull request from GitHub. to a reviewer.
Go to Github, and notice the new branch that you created. Click Compare & pull request.
Enter the description of the code change and specify the reviewer. In this example, "dlagrava" is selected.
The reviewer can be an expert or a team member who wishes to learn more about the code base.
Before clicking Create pull request, scroll down and have a good look at your code change. If you're happy, make a pull request.
When the request has been successfully made, the developer will be presented a new page from Pull requests tab.
Reviewer : Reviewer has 3 options : He/she can make comments, approve the change or make a change request.
The reviewer receives an email requesting the review. "dlagrava" visits the pull request page and clicks the link to start the review.
Click Add your review
The reviewer can click Review changes to comment/approve or request changes.
If the reviewer wishes to make a specific comment for a certain line of code, click + in the begging of the line and leave a comment and click Add single comment, this will immediately send an email to the developer.
Alternatively, if the reviewer wishes to make multiple comments, he/she may click Start a review, which will place the comment in "Pending" state, and after all comments have been made, the reviewer can click Finish your review to send the whole collection of comments in a single email.
The reviewer can finalize the review by clicking Submit review with 3 options - Comment, Approve and Request changes.
Developer: Revision of the code.
If the reviewer selects Request change, the developer will receive an email with a link that takes to this page.The developer may disagree, but let's assume he/she agreed to make a necessary code change.
The developer edit the code, and does git add/commit/push.It is also recommended to respond to the reviewer comment.
Start a review and Submit review to finalize your comment and notify the reviewer.
- Reviewer: Second review.
Reviewer receives an email with a link that takes to the page showing New changes since you last viewed.
Click + or View changes to inspect. If everything looks ok, Click Review changes and Approve.
- Admin (Reviewer) : Merge the pull request
The repository admin (most of the time, the reviewer will function as the admin) will Merge the pull request.
It is also recommended for the admin to Delete branch to keep the repository tidy.
Developer : Delete the local branch
Even if the admin deletes the branch, the developer's own copy of code still has the branch and worse still, the developer may be on the branch.
Check which branch is being used.seb56@hypocentre /home/seb56/gm_sim_pkg :git branch * fix_hardcoded_python master
Switch to master branch.seb56@hypocentre /home/seb56/gm_sim_pkg :git checkout master Switched to branch 'master' Your branch is up-to-date with 'origin/master'.
I. Delete first
Delete the branchseb56@hypocentre /home/seb56/gm_sim_pkg :git branch -D fix_hardcoded_python Deleted branch fix_hardcoded_python (was 84ef6a7).
At this point, your local "master" will be behind the origin "master" which has merged the pull request. Get the latest copy of origin master.seb56@hypocentre /home/seb56/gm_sim_pkg :git pull remote: Counting objects: 1, done. remote: Total 1 (delta 0), reused 0 (delta 0), pack-reused 0 Unpacking objects: 100% (1/1), done. From github.com:ucgmsim/gm_sim_pkg 157b335..ad64bb7 master -> origin/master Updating 157b335..ad64bb7 Fast-forward wrapper/setup_remote.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
II. Pull firstseb56@hypocentre /home/seb56/gm_sim_pkg :git pull remote: Counting objects: 1, done. remote: Total 1 (delta 0), reused 0 (delta 0), pack-reused 0 Unpacking objects: 100% (1/1), done. From github.com:ucgmsim/gm_sim_pkg 157b335..ad64bb7 master -> origin/master Updating 157b335..ad64bb7 Fast-forward wrapper/setup_remote.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)).
Delete the branch. Note the "-d" switch instead of "-D".seb56@hypocentre /home/seb56/gm_sim_pkg :git branch -d fix_hardcoded_python Deleted branch fix_hardcoded_python (was 84ef6a7
Related articles
https://help.github.com/articles/about-pull-requests/
https://help.github.com/articles/merging-a-pull-request/