4
participation points
Admin iP → Getting gready for PR reviews
This task is worth 2x2=4
participtaion points.
Textbook Git & GitHub → Reviewing PRs
The PR review stage is a dialog between the PR author and members of the repo that received the PR, in order to refine and eventually merge the PR.
Given below are some steps you can follow when reviewing a PR.
1. Locate the PR:
2. Read the PR description. It might contain information relevant to reviewing the PR.
3. Click on the Files changed tab to see the diff view.
4. Add review comments:
5. Submit the review:
Overall, I found your code easy to read for the most part except a few places
where the nesting was too deep. I noted a few minor coding standard violations
too. Some of the classes are getting quite long. Consider splitting into smaller
classes if that makes sense.
LGTM
is often used in such overall comments, to indicate Looks good to merge
.nit
is another such term, used to indicate minor flaws e.g., LGTM, almost. Just a few nits to fix.
.Approve
, Comment
, or Request changes
option as appropriate and click on the Submit review button.Step 1 Note these additional guidelines:
Comment
(i.e., not Approve
or Request changes
)Step 2 Do the first PR review as follows.
iP PR review allocation
Tutorial | Reviewer | First PR to review | Backup PR to review |
---|---|---|---|
CS2113-F10 | beaniestanley | Chiamjiaen | bryanwhl |
CS2113-F10 | CabbageTime | blank-bank | justinaquak |
CS2113-F10 | heyjinwei | song0180 | jhjhajh |
CS2113-F10 | jadenwjh | CabbageTime | blank-bank |
CS2113-F10 | song0180 | jhjhajh | cloudy3 |
CS2113-F10 | lihaoyangML | jadenwjh | CabbageTime |
CS2113-F10 | lowwilliam | baggiiiie | NoorSarrah |
CS2113-F10 | cloudy3 | lihaoyangML | jadenwjh |
CS2113-F10 | baggiiiie | NoorSarrah | beaniestanley |
CS2113-F10 | PingruiLi | nivikcivik | huachen24 |
CS2113-F10 | warmwhalefy | BlubberMonster | AlexanderTanJunAn |
CS2113-F10 | justinaquak | jianningzhuang | warmwhalefy |
CS2113-T10 | joohwan58 | hussain1998 | Vinci-Hu |
CS2113-T10 | SimJJ96 | 951553394 | violinyap |
CS2113-T10 | leeyp | brynagoh | ongweisheng |
CS2113-T10 | JoviYeung92 | zhangyongzhe20 | Cocokkkk |
CS2113-T10 | nagiteja | KimIdeas8 | Rye98 |
CS2113-T10 | kwokyto | kangxinwang | tehtea |
CS2113-T10 | tehtea | Zufiqqar | JonathanKhooTY |
CS2113-T10 | violinyap | joohwan58 | hussain1998 |
CS2113-T10 | L-Irvin | JoviYeung92 | zhangyongzhe20 |
CS2113-T10 | chenling1022 | Rizavur | iamakilahamed |
CS2113-T10 | Zufiqqar | JonathanKhooTY | geezzzyyy |
CS2113-T10 | geezzzyyy | chenling1022 | Rizavur |
CS2113-T10 | zhangyongzhe20 | Cocokkkk | sarzorwyn |
CS2113-T10 | boonjuey | KevinNgWK | soepaingzaw |
CS2113-T10 | KevinNgWK | soepaingzaw | E00426142 |
CS2113-T10 | Tyuanyuan | fangxinjia0203 | rageqqq |
CS2113-T10 | brynagoh | ongweisheng | boonjuey |
CS2113-T10 | cswbibibi | douglaslewpc | kwokyto |
CS2113-T10 | kangxinwang | tehtea | Zufiqqar |
CS2113-T10 | Vinci-Hu | Tyuanyuan | fangxinjia0203 |
CS2113-W10 | FarmZH98 | aliciatay-zls | leowxx |
CS2113-W10 | jalvinchan | LJ-37 | Emkay16 |
CS2113-W10 | jovanhuang | jonahtwl | Chihui8199 |
CS2113-W10 | JethroPhuah | LeeHanYongAndy | seangoats |
CS2113-W10 | NgManSing | isaharon | ManikaHennedige |
CS2113-W10 | ManikaHennedige | 8kdesign | yanli1215 |
CS2113-W10 | s-t-e-f | JethroPhuah | LeeHanYongAndy |
CS2113-W10 | yanli1215 | hiongkaihan | s-t-e-f |
CS2113-W10 | averliz | fsgmhoward | oscarlai1998 |
CS2113-W10 | yyixue | averliz | fsgmhoward |
CS2113-W10 | lamzf1998 | H-horizon | gerardtwk |
CS2113-W10 | Chihui8199 | kewenlok | brandonfoong |
CS2113-W10 | seangoats | yyixue | averliz |
CS2113-W10 | e0699194 | jovanhuang | jonahtwl |
CS2113T-F08 | AlexanderTanJunAn | lowwilliam | baggiiiie |
CS2113T-F08 | BlubberMonster | AlexanderTanJunAn | lowwilliam |
CS2113T-F08 | blank-bank | justinaquak | jianningzhuang |
CS2113T-F08 | huachen24 | EmilyTJX | hazelhedmine |
CS2113T-F08 | jhjhajh | cloudy3 | lihaoyangML |
CS2113T-F08 | EmilyTJX | hazelhedmine | vvvvh123 |
CS2113T-F08 | hazelhedmine | vvvvh123 | Krithigha24 |
CS2113T-F08 | NoorSarrah | beaniestanley | Chiamjiaen |
CS2113T-F08 | nivikcivik | huachen24 | EmilyTJX |
CS2113T-F08 | Krithigha24 | vaiish371 | liping-eng |
CS2113T-F08 | liping-eng | zihan9485 | heyjinwei |
CS2113T-F08 | MingShun98 | PingruiLi | nivikcivik |
CS2113T-F08 | Chiamjiaen | bryanwhl | MingShun98 |
CS2113T-F08 | vaiish371 | liping-eng | zihan9485 |
CS2113T-F08 | zihan9485 | heyjinwei | song0180 |
CS2113T-F08 | vvvvh123 | Krithigha24 | vaiish371 |
CS2113T-F08 | jianningzhuang | warmwhalefy | BlubberMonster |
CS2113T-F08 | bryanwhl | MingShun98 | PingruiLi |
CS2113T-T09 | wjchoi0712 | limwenfeng | L-Irvin |
CS2113T-T09 | fangxinjia0203 | rageqqq | wjchoi0712 |
CS2113T-T09 | JonathanKhooTY | geezzzyyy | chenling1022 |
CS2113T-T09 | iamakilahamed | leeyp | brynagoh |
CS2113T-T09 | hussain1998 | Vinci-Hu | Tyuanyuan |
CS2113T-T09 | limwenfeng | L-Irvin | JoviYeung92 |
CS2113T-T09 | soepaingzaw | E00426142 | SimBowen |
CS2113T-T09 | sarzorwyn | cswbibibi | douglaslewpc |
CS2113T-T09 | Cocokkkk | sarzorwyn | cswbibibi |
CS2113T-T09 | Rye98 | SimJJ96 | 951553394 |
CS2113T-T09 | Rizavur | iamakilahamed | leeyp |
CS2113T-T09 | SimBowen | zikunz | nagiteja |
CS2113T-T09 | rageqqq | wjchoi0712 | limwenfeng |
CS2113T-T09 | 951553394 | violinyap | joohwan58 |
CS2113T-T09 | KimIdeas8 | Rye98 | SimJJ96 |
CS2113T-T09 | zikunz | nagiteja | KimIdeas8 |
CS2113T-T09 | ongweisheng | boonjuey | KevinNgWK |
CS2113T-T09 | E00426142 | SimBowen | zikunz |
CS2113T-T09 | douglaslewpc | kwokyto | kangxinwang |
CS2113T-W09 | brandonfoong | xseh | marklowsk |
CS2113T-W09 | gerardtwk | NgManSing | isaharon |
CS2113T-W09 | kewenlok | brandonfoong | xseh |
CS2113T-W09 | LJ-37 | Emkay16 | lamzf1998 |
CS2113T-W09 | hiongkaihan | s-t-e-f | JethroPhuah |
CS2113T-W09 | LeeHanYongAndy | seangoats | yyixue |
CS2113T-W09 | jonahtwl | Chihui8199 | kewenlok |
CS2113T-W09 | tzexern | e0699194 | jovanhuang |
CS2113T-W09 | ivanchongzhien | fupernova | jalvinchan |
CS2113T-W09 | H-horizon | gerardtwk | NgManSing |
CS2113T-W09 | marklowsk | ivanchongzhien | fupernova |
CS2113T-W09 | leowxx | tzexern | e0699194 |
CS2113T-W09 | isaharon | ManikaHennedige | 8kdesign |
CS2113T-W09 | oscarlai1998 | FarmZH98 | aliciatay-zls |
CS2113T-W09 | fsgmhoward | oscarlai1998 | FarmZH98 |
CS2113T-W09 | aliciatay-zls | leowxx | tzexern |
CS2113T-W09 | Emkay16 | lamzf1998 | H-horizon |
CS2113T-W09 | fupernova | jalvinchan | LJ-37 |
CS2113T-W09 | xseh | marklowsk | ivanchongzhien |
CS2113T-W09 | 8kdesign | yanli1215 | hiongkaihan |
If the student you have been allocated to review has not created a PR (or the PR has a trivial amount of code), you can review the Backup PR to review provided in the allocation table. Failing both, review another PR allocated to another student in your own tutorial but not in your team.
Tip for future reference: GitHub allows you to filter PRs/Issues using various criteria such as author:AuthorUsername
(example -- see the filters
text box in the target page).
Alternatively, you can use PR labels (if any) to filter PRs/Issues.
FAQ: How many comments should I add? Answer: Depends on the code being reviewed but we expect most PRs would warrant at least 4-5 comments. If the PR is huge, you can stop when you think you've put in a fair amount of time on the job (~15 minutes) and added enough comments for the PR author to receive some value.
iP PR review allocation
Tutorial | Reviewer | Second PR to review | Backup PR to review |
---|---|---|---|
CS2113-F10 | beaniestanley | MingShun98 | PingruiLi |
CS2113-F10 | CabbageTime | jianningzhuang | warmwhalefy |
CS2113-F10 | heyjinwei | cloudy3 | lihaoyangML |
CS2113-F10 | jadenwjh | justinaquak | jianningzhuang |
CS2113-F10 | song0180 | lihaoyangML | jadenwjh |
CS2113-F10 | lihaoyangML | blank-bank | justinaquak |
CS2113-F10 | lowwilliam | beaniestanley | Chiamjiaen |
CS2113-F10 | cloudy3 | CabbageTime | blank-bank |
CS2113-F10 | baggiiiie | Chiamjiaen | bryanwhl |
CS2113-F10 | PingruiLi | EmilyTJX | hazelhedmine |
CS2113-F10 | warmwhalefy | lowwilliam | baggiiiie |
CS2113-F10 | justinaquak | BlubberMonster | AlexanderTanJunAn |
CS2113-T10 | joohwan58 | Tyuanyuan | fangxinjia0203 |
CS2113-T10 | SimJJ96 | joohwan58 | hussain1998 |
CS2113-T10 | leeyp | boonjuey | KevinNgWK |
CS2113-T10 | JoviYeung92 | sarzorwyn | cswbibibi |
CS2113-T10 | nagiteja | SimJJ96 | 951553394 |
CS2113-T10 | kwokyto | Zufiqqar | JonathanKhooTY |
CS2113-T10 | tehtea | geezzzyyy | chenling1022 |
CS2113-T10 | violinyap | Vinci-Hu | Tyuanyuan |
CS2113-T10 | L-Irvin | Cocokkkk | sarzorwyn |
CS2113-T10 | chenling1022 | leeyp | brynagoh |
CS2113-T10 | Zufiqqar | chenling1022 | Rizavur |
CS2113-T10 | geezzzyyy | iamakilahamed | leeyp |
CS2113-T10 | zhangyongzhe20 | cswbibibi | douglaslewpc |
CS2113-T10 | boonjuey | E00426142 | SimBowen |
CS2113-T10 | KevinNgWK | SimBowen | zikunz |
CS2113-T10 | Tyuanyuan | wjchoi0712 | limwenfeng |
CS2113-T10 | brynagoh | KevinNgWK | soepaingzaw |
CS2113-T10 | cswbibibi | kangxinwang | tehtea |
CS2113-T10 | kangxinwang | JonathanKhooTY | geezzzyyy |
CS2113-T10 | Vinci-Hu | rageqqq | wjchoi0712 |
CS2113-W10 | FarmZH98 | tzexern | e0699194 |
CS2113-W10 | jalvinchan | lamzf1998 | H-horizon |
CS2113-W10 | jovanhuang | kewenlok | brandonfoong |
CS2113-W10 | JethroPhuah | yyixue | averliz |
CS2113-W10 | NgManSing | 8kdesign | yanli1215 |
CS2113-W10 | ManikaHennedige | hiongkaihan | s-t-e-f |
CS2113-W10 | s-t-e-f | seangoats | yyixue |
CS2113-W10 | yanli1215 | JethroPhuah | LeeHanYongAndy |
CS2113-W10 | averliz | FarmZH98 | aliciatay-zls |
CS2113-W10 | yyixue | oscarlai1998 | FarmZH98 |
CS2113-W10 | lamzf1998 | NgManSing | isaharon |
CS2113-W10 | Chihui8199 | xseh | marklowsk |
CS2113-W10 | seangoats | fsgmhoward | oscarlai1998 |
CS2113-W10 | e0699194 | Chihui8199 | kewenlok |
CS2113T-F08 | AlexanderTanJunAn | NoorSarrah | beaniestanley |
CS2113T-F08 | BlubberMonster | baggiiiie | NoorSarrah |
CS2113T-F08 | blank-bank | warmwhalefy | BlubberMonster |
CS2113T-F08 | huachen24 | vvvvh123 | Krithigha24 |
CS2113T-F08 | jhjhajh | jadenwjh | CabbageTime |
CS2113T-F08 | EmilyTJX | Krithigha24 | vaiish371 |
CS2113T-F08 | hazelhedmine | vaiish371 | liping-eng |
CS2113T-F08 | NoorSarrah | bryanwhl | MingShun98 |
CS2113T-F08 | nivikcivik | hazelhedmine | vvvvh123 |
CS2113T-F08 | Krithigha24 | zihan9485 | heyjinwei |
CS2113T-F08 | liping-eng | song0180 | jhjhajh |
CS2113T-F08 | MingShun98 | huachen24 | EmilyTJX |
CS2113T-F08 | Chiamjiaen | PingruiLi | nivikcivik |
CS2113T-F08 | vaiish371 | heyjinwei | song0180 |
CS2113T-F08 | zihan9485 | jhjhajh | cloudy3 |
CS2113T-F08 | vvvvh123 | liping-eng | zihan9485 |
CS2113T-F08 | jianningzhuang | AlexanderTanJunAn | lowwilliam |
CS2113T-F08 | bryanwhl | nivikcivik | huachen24 |
CS2113T-T09 | wjchoi0712 | JoviYeung92 | zhangyongzhe20 |
CS2113T-T09 | fangxinjia0203 | limwenfeng | L-Irvin |
CS2113T-T09 | JonathanKhooTY | Rizavur | iamakilahamed |
CS2113T-T09 | iamakilahamed | ongweisheng | boonjuey |
CS2113T-T09 | hussain1998 | fangxinjia0203 | rageqqq |
CS2113T-T09 | limwenfeng | zhangyongzhe20 | Cocokkkk |
CS2113T-T09 | soepaingzaw | zikunz | nagiteja |
CS2113T-T09 | sarzorwyn | kwokyto | kangxinwang |
CS2113T-T09 | Cocokkkk | douglaslewpc | kwokyto |
CS2113T-T09 | Rye98 | violinyap | joohwan58 |
CS2113T-T09 | Rizavur | brynagoh | ongweisheng |
CS2113T-T09 | SimBowen | KimIdeas8 | Rye98 |
CS2113T-T09 | rageqqq | L-Irvin | JoviYeung92 |
CS2113T-T09 | 951553394 | hussain1998 | Vinci-Hu |
CS2113T-T09 | KimIdeas8 | 951553394 | violinyap |
CS2113T-T09 | zikunz | Rye98 | SimJJ96 |
CS2113T-T09 | ongweisheng | soepaingzaw | E00426142 |
CS2113T-T09 | E00426142 | nagiteja | KimIdeas8 |
CS2113T-T09 | douglaslewpc | tehtea | Zufiqqar |
CS2113T-W09 | brandonfoong | ivanchongzhien | fupernova |
CS2113T-W09 | gerardtwk | ManikaHennedige | 8kdesign |
CS2113T-W09 | kewenlok | marklowsk | ivanchongzhien |
CS2113T-W09 | LJ-37 | H-horizon | gerardtwk |
CS2113T-W09 | hiongkaihan | LeeHanYongAndy | seangoats |
CS2113T-W09 | LeeHanYongAndy | averliz | fsgmhoward |
CS2113T-W09 | jonahtwl | brandonfoong | xseh |
CS2113T-W09 | tzexern | jonahtwl | Chihui8199 |
CS2113T-W09 | ivanchongzhien | LJ-37 | Emkay16 |
CS2113T-W09 | H-horizon | isaharon | ManikaHennedige |
CS2113T-W09 | marklowsk | jalvinchan | LJ-37 |
CS2113T-W09 | leowxx | jovanhuang | jonahtwl |
CS2113T-W09 | isaharon | yanli1215 | hiongkaihan |
CS2113T-W09 | oscarlai1998 | leowxx | tzexern |
CS2113T-W09 | fsgmhoward | aliciatay-zls | leowxx |
CS2113T-W09 | aliciatay-zls | e0699194 | jovanhuang |
CS2113T-W09 | Emkay16 | gerardtwk | NgManSing |
CS2113T-W09 | fupernova | Emkay16 | lamzf1998 |
CS2113T-W09 | xseh | fupernova | jalvinchan |
CS2113T-W09 | 8kdesign | s-t-e-f | JethroPhuah |
If the allocated PR is not suitable, use the same strategy as before to find an alternative PR to review.