Introduction
A big part of open-source is working with others, and I wanted to gain experience with projects created by others and also wanted to get feedback on the open source project I had created earlier (cli-ssg). Looking for someone to peer review with, I ended up connecting with DerekJxy who had created My-First-SSG. Similar to the tool I created, he had also built a SSG using JavaScript. Since his tool used the same programming language, the code review process went quite smoothly for me.
Reviewing My-First-SSG
I started off by installing the tool after going through the documentation. I then tried running the tool to test its functionality and I ended up finding quite a few problems. I used the Issues tab in his GitHub repo to file a total of 5 issues highlighting these problems:
Unable to use files as input using -i
As per the repo readme, node server.js -i '.\Silver Blaze.txt' should produce aSilver Blaze.htmlin./dist`. However when I ran it, the tool failed to generate HTML and logged errors in the console.
Html is generated in the default location even if an output directory is specified
The repo readme as well as --help for the tool highlighted an option to use -o or –output to specify an output directory. However, the tool kept using the default ./dist even if I supplied an output directory.
Errors in console when directories are present in the input path
Everything in the input path was being directly passed to readFile and thus, if the input directory contained directories, it would spit errors in console.
Non .txt files are not ignored/skipped
The tool even generated HTML for non .txt files such as .json, .md etc
Generated files fail HTML5 validator
The generated HTML files failed the validator at https://html5.validator.nu/
Going through Issues filed in my repo
As a part of the peer review process, DerekJxy also tested and reviewed cli-ssg. He filed 4 issues on the repo, of which I found one to be useful (fixed and closed!):
Unused variables
One issue highlighted that I had 2 imports in one of my files that were not needed. It seemed like these had lingered on when I had just started, so I ended up removing these 2 lines based on this feedback.
Issue 3 and Issue 4
Two of the issues were about code improvement. However I did not see the suggestions offering any improvement, one(#3) of them would rather hinder upgradability if more functionality in parsing the input file is added.
Unnecessary Statement
The last issue suggested that a part of code should be removed because the output directory would always have a default. However, that code was crucial in validating the output path set by the user and it wasn’t related to the default path.
Conclusion
Overall, I ended up merging a pull-request to remove the 2 imports which closed the first issue. For the remaining 3, I highlighted why the suggestion was not useful and ended up closing them. Thus, I was able to close all the issues filed on my repo after acting upon the feedback.
Top comments (0)