For week 3 of my course Topics in Open Source Development at Seneca College, I was tasked with working a partner to review each other's code. We looked at each other's repositories:

My repository: https://github.com/paulkim26/til-to-html

My partner's repository: https://github.com/mismathh/TILerator

Identifying Tasks

The first step was to submit an issue; I intended to enhance mismathh's tool to:

• Handle Markdown (.md) files
• Parse Markdown bold text as html <b> tags

I looked over the code and realized there were multiple ways to approach this. I could simply append the new functionality to the existing files or I could make a new module that encapsulated the new functionality. Then that lead to more questions like "should I create a new folder to contain my new code" or "what should I name these new files/folders". I asked and mismathh thankfully clarified I should implement the functionality within a new module. Had I not asked, I could've made a bad assumption I'd have to revert down the line.

Contributing Code

I forked the repository and started working on a new branch. I was careful to refrain from changing the code too drastically, changing things that were outside of the scope of my submitted issue, and adhering to mismathh's coding style - even if I personally did things differently in my own projects.

For example, I noticed there were a lot of index based for loops where I would have personally opted to use for...of loops or use methods like forEach, filter, map, etc.

Regardless, the code was very readable; I commend mismathh for coding in a manner that was straightforward and self-documenting with additional comments inserted where needed. I added a new file called markdownToHtml.js that encapsulated my new markdown conversion functionality and imported it into the existing utils.js file - that helped keep things readable.

When I was ready, I pushed my branch to GitHub and created a new pull request. It felt a bit repetitive to describe what I had changed all over again and reiterate my commit messages in the pull request's description, but I suppose there's no harm in being too verbose and explicit with my intentions, especially since this was not my codebase to start with.

Pull Request Feedback

mismathh looked over my pull request and responded with a request for a change - it turned out that I had introduced a new bug.

I was using the split method to extract the file type from a file path name, for example:

const filePath = "examples/til.txt";
const fileType = filePath.split(".")[1];

In this case, fileType would correctly return "txt" as expected by splitting the filePath string by the "." character and returning the string that was after it.

However, this was predicated on filePath being written with 1 period - this was not a robust solution.

mismathh noted this would not work for all situations:

const filePath = "./examples/til.txt";
const fileType = filePath.split(".")[1];

In this example, fileType would resolve to "/examples/til" rather than "txt" as expected. He suggested using the path.extname method which was explicitly designed to extract the extension name from a file path. This proved to be a much more robust solution that worked great! I implemented his solution accordingly.

Pull Request - Lessons Learned

From this experience, I learned to:

• Use existing methods/functions/APIs that are custom suited for the problems I'm trying to solve if possible.
• Consider how others may test and run the same code differently from me.

Next time, I should think more carefully about a bug report that may appear to be a false positive to me and see how I may be biased in my perception of it. mismathh pointed out that I should use an index of value 2 (const fileType = filePath.split(".")[2];) - this should've tipped me off that there were test cases I had not accounted for instead of assuming mismathh had a mistake. He had gone out of his way to report this; there was a user story I had yet to discover.

Receiving a pull request

On the opposite end of this experience, mismathh submitted a pull request to my repository. I really appreciated how explicitly mismathh detailed his planned changes to my code, that alleviated a lot of my initial reservations about having someone touch up my work.

For this pull request, he submitted 8 commits, I really appreciate that not only did he split his commits into digestible, sensible scopes, but he inferred my commit naming convention (Conventional Commits) despite me not stipulating so and adhered to it. In hindsight I should've made a note in my README about following this naming convention - I got lucky that mismathh is just that awesome.

Reviewing the Pull Request

I pulled the new branch into my local machine and started by just running the code through all of my test cases. Everything worked great. Then I checked all of the new code - mismathh was careful to adhere to my style and project structure, even separating his new markdown header parsing function into its own module, as I had similarly done.

All in all, the changes were solid, they worked great and were readable. I made a couple of suggestions that adhered to my stylistic preferences such as:

• Using const instead of let whenever possible
• Eliminating a redundant iteration in a for loop
• Renaming a variable
• Eliminating a redundant template string

So, very small changes. He quickly implemented all of my suggestions and I accepted and merged his pull request.

In my head I was thinking "I could just accept this pull request without revisions and call it a day" but I'm glad I didn't and asked for the changes I would've made on my own. mismathh was receptive to feedback and I could simply move on to other endeavours after the pull request was submitted.

Thanks for reading!