DEV Community

Gulnur Baimukhambetova
Gulnur Baimukhambetova

Posted on • Updated on

My first code review and issues!

Hello everyone! ๐Ÿ™‹๐Ÿปโ€โ™€๏ธ

Today, we decided to review each other's code with my friend who also developed his version of the SSG command-line tool (pls see my previous post for details). I found him in one of my courses' slack channels during the previous week and direct messaged him with a request to partner. Luckily, he turned out to be a very kind soul and not only accepted my request but also verbally supported me almost every day afterwards and we became friends. Otherwise, I would have had a fear to message random people on slack for the rest of my life. I am an extravert, but not when it comes to group chats... ๐Ÿ˜ฎโ€๐Ÿ’จ

As both of our projects were on similar topics, it was quite easy to understand how the tool works and what are the instructions. At first, the tool seemed working and I could not think of any issues, but when I checked the code and started reading through every line, I started having ideas for improvement. Remembering the struggles and some error handling techniques from my project also helped me identify which ones my partner could have been missing.

So, these were the issues I filed on my friend's repo:

  • Issue #1 which is basically a documentation improvement.
  • Issue #2 which is also an improvement but for files that are currently uploaded into the GitHub repo. The suggestion is to get rid of the testing files as they are not directly connected to the solution itself and would be installed to the user's machine for no reason.
  • Issue #3 which is a code improvement - an idea on how to shorten the code and avoid repetition.
  • Issue #4 which is also a code improvement for error handling. Right now, if the input file is unsupported, nothing is done to let the user know. Suggestion is to at least log the error.
  • Issue #5 which is a bug connected to the use of files removal function choice. It only works for files and does not consider sub-directories which are possible in some use cases.

Well, my friend also found several improvement ideas and bugs in my solution. So, let's discuss them:

  • Issue #1 is a bug connected to file reading and quotation marks. Basically, all quotation marks needed to be converted into the escape characters in order to be processed correctly and not as end of the string.

  • Issue #2 is a bug which I did not know was a requirement. However, still seemed better to have it. It says that the older output folder needs to be deleted if another output folder has been chosen.

  • Issue #3 is a bug only because we had instructions strictly specifying what each option should do. My version option only showed version, while it needs to display the tool name as well.

  • Issue #4 is an improvement for better readability of the website for different stylesheets. I used the background colour which imitated the old paper feeling but it sometimes did not work with other styling options.

  • Issue #5 is a code improvement for retrieving a value from json object.

All of the issues were fixed and the tool was updated the same day.

It was actually so great to have someone review your code. First, at least now I know that it works for sure not only on my machine but on others. Plus, all of the issues filed we valid improvement ideas and bugs, which I could have missed myself. So, I think it was a very useful activity.

YAY!

Top comments (0)