For our first lab of the OSD 600 course, we were asked to find a partner and test and review each other’s code. This was the first time I filed issues on GitHub and learned a lot in the process. For this task, Batuhan and I teamed up to help review and test each other’s code. Batuhan is an excellent programmer and we have done many projects together in the past, including winning two hackathons together this year, presenting our research findings to the Seneca Business Week Conference, and working at Seneca Innovation to help out an industry partner of Seneca College.
The Testing and Reviewing Process
This was the first time I tested and reviewed someone else’s code so attentively. My partner did an excellent job building his Static Site Generator in C++ and used CMake to automate the compilation process and to be able to use external libraries (argparse, termcolor).
The ‘make prepare’ command made it so easy and seamless for me to download all necessary dependencies that I did not have to go about figuring things out on my own. The documentation was clear – all I had to do was follow the instructions and run the project without any hiccups. I did, however, ask my partner to explain a little bit more about CMake for users unfamiliar with it.
Having My Code Reviewed
It was amazing to see all the little issues that another pair of eyes could catch that I so easily neglected. My partner identified and filed five issues on my code.
Issue 1 - Inconsistencies in the titles output in the terminal
I already knew this issue would come up because I am still in the process of solving this. I have not yet come up with a solution, but I am hoping to resolve it very soon before release 0.1 is due.
Issue 2 - Color scheme and readability issue in default static site
I am glad my partner pointed out this issue because readability and the aesthetics of a website is important to me. It was an easy fix as I specified a default stylesheet that changed the entire look of my static site.
Issue 3 – Improving the documentation and making it clearer by specifying the different features available
As stated in the issue, I improved the documentation along with a list of features that my SSG is packed with. I believe it is best practice to have a well laid out table displaying all the flags for the user. A list of features also makes it easy for anyone going through my repo to understand what the project offers.
Issue 4 - Exception handling on file extensions
Currently, my program does not display any error message or generate any output if the input files are anything other than a .txt file. It would be clearer for the user if an error message is an output in case any other file extensions were input. This is a great point and important to work on.
Issue 5 – Providing a URL for the logo instead of keeping it in the folder
This was a good point brought up in the review process. I was able to generate a URL with Imgur for the picture so that the logo did not have to reside within the project folder. Having just a picture in a folder is quite redundant
Reviewing My Partner’s Code
My partner’s code was implemented well, and I could tell that he paid attention to efficiency, readability and had sufficient comments for me to understand what the code was doing. I learned a lot in terms of implementation style and especially CMake. I asked him a lot of questions in terms of his rationale for using CMake, how he learned about CMake and how using CMake vs not using it does. Through our conversations, I learned a lot. Here is a link to Batuhan's blog about our detailed discussion on it.
There were a few minor issues I found in the code listed below.
Issue 1 - Error message for any other files other than .txt files
If the input file is not .txt, I asked my partner to display a message saying the raw data files other than .txt format are not supported under the current version. Currently, it generates no output if provided with any other file extensions. This might confuse the user or leave them wondering what went wrong. I also had this same issue that I am working on fixing.
Issue 2 - Provide instructions for Windows and other Operating System user files
The current README contains setup instructions only for macOS users.
The README could be improved by providing instructions for other operating systems such as Windows as well.
Issue 3 - HTML file indentation issue in the dist directory
The HTML files in the output dist directory were not formatted with proper indentations. It was hard to read, and I asked my partner to identify a solution for that issue to make it more readable for the user.
Issue 4 - Enhancement of modularity by moving the argparse and termcolor in src directory
In the app/main.cpp, the argparse dependency and termcolor were making the main.cpp look cluttered. It is best practice to have only the main function in main.cpp. Moving the argparse and termcolor in src directory would make the project more modular.
Issue 5 – More documentation on CMake configurations
I asked my partner to provide more explanations about CMake configurations. For example, what does the config folder actually do? And how does CMake help you to configure external dependencies? This could make your README more powerful and help users a lot!
This entire process of testing, reviewing and filing issues made me think of the code very critically. As someone who just started open-source development, I would say it went well. This gave me a real life experience because I am well aware that in the job environment I would have to read and understand other people's code all the time. I know I have so much to improve upon in terms of providing better feedback and comments. I was not able to comment much on the efficiency of the implementation and I am sure there are things I overlooked that could be caught by more experienced programmers. But that is all part of the learning curve and I am so glad to have the opportunity to start now to build such really important skills.
Oldest comments (0)