I have never had the experience of doing proper code reviews until now when I explored one of the repositories centered around the idea of refactoring code through a CLI-based tool. The abstract idea behind the tool was to get an input file, transform it through an LLM, and give the output back to the user on the terminal.
Approach
My first approach to progress toward code review was to grasp the idea and have the project set up locally on my machine. Also to get more clarity on the project's structure, I contacted the maintainer for this project via Slack. I think this type of async communication method where one doesn't need to be in constant communication with each other is better than having to converse synchronously, it provides me with the freedom and flexibility to contact the maintainer.
Once I had the repository cloned, my next focus was to get the CLI tool running and successfully execute the demo on my machine, which I eventually managed. Since the codebase was in JavaScript, I navigated through it with ease. However, while reviewing the index.js file, I was surprised to find all the functions bundled together in one place. Typically, I expect codebases to follow a more modular approach, with different tasks separated into individual files or folders, following the principle of Separation of Concerns. In this case, the entire code structure could have benefited from being divided into distinct files for better organization. Moreover, given that the program integrates with an LLM API, it would have been more efficient if the LLM-related logic were encapsulated in a separate object, further improving modularity and maintainability.
Issues
Upon reviewing the code I found a couple of issues and some parts that would benefit from minor improvements, I would elaborate on the same below
Issue 1
While inspecting the code, One thing that I found was that most of the errors were partly being logged via stderr and the other errors were just being thrown in the catch block
Some kind of uniformity to how the errors were being treated would provide a much better understanding of what or how the error is being caused and save unnecessary trouble.
Issue 2
The project utilized a library that would provide the utility of having a spinner while the API call is being completed. The idea to use the spinner was a great one but the issue here was the code was utilizing a hardcoded value to run the spinner while the API was being completed, this was leading to cases where the API call was being completed but the spinner was still running.
The suggestion here was that rather than using the hardcoded value, it would be better if the spinner starts once the call is made and then stops once the result is achieved or displays an error if nothing is received from the API call.
Issue 3
This wasn't an issue per se, but rather a suggestion to improve the maintainability of the code. The project allowed users to specify the model that they wanted to use for the LLM API, now how the argument was being handled was using a temporary array and using ternary operators to choose the model, I found this to be inefficient and hard to understand.
The suggestion here was to utilize a Map to link the options to the model name that is being sent to the API, this would firstly improve the maintainability of the code and secondly allow for more model support in the future.
Fixing Issues In My Codebase
Just like a Jedi in a Galaxy far, far away, I embarked on a quest to fix issues in my codebase after opening issues in distant codebases haha.
To give a quick background, my codebase is a CLI tool that transpiles code from one language to another, including C++, Java, Python, or JavaScript, and returns back all the files in a new directory called transpiledFiles. The code is written in Typescript and uses Bun as the environment (fun fact, this is my first time using a project that uses Bun as the environment and package installer, P.S. I am starting to like Bun now haha)
Now, let's dive into how I tackled the issues that were opened in my repository.
Issue Fix 1
The issue that was raised here was related to the binary for the code not being able to run on different machines from mine this was due to the path variable not having the bun binary, so to solve this issue I added the instructions in the README.md file.
Issue Fix 2
This issue addresses the confusion about the usage of the -o option in the CLI tool, according to the guidelines specified, the -o option should've been used to specify the output files but I used it to specify the type of language that the code would be transpiled to.
So to fix this issue I made a change to use the -l(shorthand for --language) option rather than using the -o option.
Issue Fix 3
This is by far one of the most interesting issues that I have worked on as this required me to reproduce the issue on a Virtual Machine and go through it step by step to see what I was doing wrong
The issue was centered around the idea of the binary still not being able to run even though the bun link command was still being used
The issue here for me was that, for some reason, the binary for the program was working on my computer and it was installed there, so to reproduce it I would have to either remove the only working code from my local machine or clone the repo on a VM and try to reproduce it.
As an experienced programmer (kinda XD), I would not remove any part that was working from my local machine, So the only option left for me was to run it on a virtual machine
I was successful in reproducing the issue, and then I was quickly able to find the solution for the same as well, it turns out that before using the bun link command, the files were not being transpiled from typescript to javascript, so in short, the bun run build command was not being run.
So I included the instructions for the same in the README.md file and tested it out on my virtual machine and it worked.
Personal Experience
I have found that the task of code reviewing is often more tiresome than fixing issues in your own codebase. This could partly be due to the time-consuming nature of going through an entire codebase at once during reviews. In contrast, when solving issues in your own code, the aspect of isolating problems comes into play, allowing you to target specific sections that need fixing. This focused approach typically makes the task feel less overwhelming and more manageable.
Nevertheless, I did find that reviewing code written by other developers provided me with new aspects to writing code and linking logic together, it was a fun experience.
Top comments (0)