DEV Community

Avelyn Hyunjeong Choi
Avelyn Hyunjeong Choi

Posted on • Updated on

My first code review in open-source community

Q1. How did you go about finding someone to work with?

For this project, I used a Slack channel to find someone to collaborate with. I came across another student who had finished release 0.1, so I sent him a direct message through Slack to ask if I could review and test his code. He agreed, and that's where I began.

Q2. What was it like testing and reviewing someone else's code? Did you run into any problems? Did anything surprise you?

It was a lot harder than I thought because when you're the code owner, you intimately know what you wrote. However, I needed to delve deep and thoroughly review the code in order to grasp the intent behind it. So, what I did was go through the code line by line, testing it simultaneously to ensure I didn't miss anything.

Q3. What was it like having someone test and review your code? Were you surprised by anything?

It was a valuable learning experience. I was surprised to discover mistakes that I did not catch during my initial review, and it also gave me new insights and perspectives on my code, leading to better solutions.

Q4. What kind of issues came up in the testing and review? Discuss a few of them in detail.

While testing another student's code, I noticed that a crucial functionality was missing, and the README.md did not provide specific instructions to address this issue. For instance, users should have the ability to specify the CSS stylesheet URL using an optional argument. However, only the default CSS was applied, and users were not given the option to apply custom CSS to the generated HTML file.

Q5. Provide links to issues you filed, and discuss what you found.

- Expected to allow users to optionally specify a stylesheet URL with --stylesheet or -s, but default CSS stylesheet to be used.
- There is no feature for the user to specify the URL themselves.
Enter fullscreen mode Exit fullscreen mode
- Once you update the code to allow users to specify the CSS stylesheet URL, it would be better to provide more information about this by including usages/examples.
- It can be beneficial for users if you include a few screenshots of usage with real output in the README.md. This will help users gain a better understanding of what this app does at a higher level.
Enter fullscreen mode Exit fullscreen mode
- Could you please enhance the formatting of the generated HTML files?
- Currently, the generated HTML file consists of one long line instead of having a proper format. Could you please make an improvement on this for a better user experience?
Enter fullscreen mode Exit fullscreen mode
- When you need to extract only the filename from the file path, you can use the basename() method. This function returns only the filename, so you don't need to use split in this case.
Enter fullscreen mode Exit fullscreen mode
Line 18 in index.js

When you use const packageJson = require("../package.json"); to retrieve the package.json file instead of doing fs.readFile(...), you don't have to parse it, which is an extra step.
Enter fullscreen mode Exit fullscreen mode

Q6. Provide links to issues that were filed on your repo, and what they were about.
Issue 1

In the generated HTML files, the <h1> tags appear outside the <body> tag. This is a non-standard HTML structure. The <h1> tags should be correctly placed within the <body> tag to maintain proper document structure.
Enter fullscreen mode Exit fullscreen mode

Issue 2

The documentation mentions that users can specify either a file or folder as input for conversion. It's not clear how the tool handles scenarios where a file and a folder share the same name. Clearer guidelines or error messages in such cases would help in improving user experience.
Enter fullscreen mode Exit fullscreen mode

Issue 3

The current code processes all files in the directory without checking for the .txt extension. Before processing, verify the file extension to ensure it's a .txt file.
Enter fullscreen mode Exit fullscreen mode

Issue 4

The readFile function uses the type any for its parameters, which can lead to potential runtime issues and reduces the advantages of TypeScript's type-checking. The types should be more specific, e.g., inputPath: string.
Enter fullscreen mode Exit fullscreen mode

Issue 5

When specifying a stylesheet URL using the -s or --stylesheet option, there is no validation mechanism in place to check if the provided URL is valid or if it points to an actual CSS file. This can lead to broken styles in the generated HTML if an invalid URL is provided. Add a validation check for the provided CSS URL to ensure it's valid and accessible. Display a warning or error message if the URL is invalid or inaccessible.
Enter fullscreen mode Exit fullscreen mode

Q7. Were you able to fix all your issues? What was that like?

I was surprised because I believed I had thoroughly reviewed and tested my code. However, the reviewer discovered a bug. I was able to address the issues, and it provided me with another perspective to evaluate my code more objectively. Once again, I realized that no code is ever perfect.

Q8. What did you learn through the process of doing the testing and reviewing?

Since it wasn't just me reviewing and testing my code, I made an extra effort to ensure its quality. When reviewing and testing another student's code, I was especially meticulous. I worried about potentially misidentifying an issue due to a lack of understanding. It was intimidating knowing I had to locate mistakes and communicate them to another student. I also recognized that I may not necessarily be a more proficient programmer than them. How could I identify a bug and correct their mistake? However, the other student who received my review was very humble and made adjustments to their code based on my recommendations. It was a highly valuable experience as a beginner programmer.

Top comments (0)