DEV Community

Tong Liu
Tong Liu

Posted on

Reflect on Pull Request code review

This is my first-time reviewing code on Github. It was actually quite fun to read others' code because I was scrutinizing the code suspiciously while I was reading others' code, and through this inspection of code, I realized that it is not easy to do code reviews than writing code because doing code review requires us to not only know how the functions should be implemented but we also need to reading others' code and trying to understand them. On top of that, we also need to know the best practice for the code we review, which requires us to have somewhat experience in this field. However, since we can't think about every aspect of our code when we are writing them, we might make mistakes or not perform the best practice, this is the scenario where code review becomes very important for keeping a good standard in the code we submit.

What did I review?

  1. Telescope
    Link: https://github.com/Seneca-CDOT/telescope/pull/3762/files/87742e87eac0b136f6f53b0c38c2679f03317abd
    This is a PR made by Piotr to modify the documentation to provide a solution for data not showing up after the project is set up. This problem was found by me and Piotr while we are trying to work on the issue 3639. In this code review, Piotr updated the documentation for specific detailed steps we need to do after initializing the project in our local environment. I found at the end of the document he modified, a generalization of the problem and solutions is mentioned, however, the fact that the parser needs to be restarted after db:init was not mentioned, so I made a code review on this trying to amend it.

  2. MyPhotoHub
    Link: https://github.com/humphd/my-photohub/pull/9/files/f65ac86f3321207953d66b02c9b9fff1942828f1
    This PR was made to make an initial implementation for this project. In this project, I found that the Regular Repression that matches a parameter was hard-coded and has occurred more than once. I think this might not be a good practice to hard code it on every occurrence since it's not future-proof when the regular expression needs to be changed. If this Regular Expression could be referenced from a variable or configuration file, it would make the future changes on it way less headache.

Top comments (0)