DPS909 Blog - Lab 2: Contributing and Submitting Pull Requests

This week for my Open-Source course (DPS909), I completed my second lab of the semester. The purpose of this lab was to get us comfortable with contributing to other people's projects and submitting a pull request (PR) on their repo.

Background

After completing release 0.1 of my static site generator and having that code reviewed by another person in lab 1. It was time to start contributing to other projects and to learn how to create PRs. In addition, it was a chance to put our code reviewing experience from lab 1 to work by reviewing other people's PRs and code changes on our own repo.

Contributing to Another Repo

For this lab, I decided to contribute to cychu42's static site generator project. So, I forked his project into my own repo and began working on it

New Feature Added

The new feature I decided to add was a Markdown (MD) parser. I created an issue in the repo to announce the new feature I wanted to see and that I intended to work on it. Originally, the tool parses text files and generates an HTML file based on the text file. I wanted to add a similar feature but for MD files. In addition, the MD parser will also parse bolded text and display the bolded text in the HTML file.

Code Changes

• I added a new function, mdReader(source), to the source code in SSC.js that is used to convert MD files into HTML.
  • The repo had already contained a function, txtReader(source), that converts text files to HTML. This function is similar, but adds bold (<b>) HTML tags when it encounters either ** or __ in the MD file.

• Since mdReader(source) is very similar to an existing function, I also created two more functions to improve code reusability.

  • startHTML(title, css) creates the first half of the HTML file for both MD (in mdReader(source)) and text (in txtReader(source)) files. So, it makes sense to put this into a function in order to be able to reuse the logic.
  • interface(inStream) creates the reader interface for reading from a file. So again, it makes to put this logic in a function since both text and MD reader functions need the same logic.

• I also added the path module to improve file path validation in the code.

  • The logic for determining if a path is a text file, MD file, or a directory was changed to make use of the module by reading the file extension.
  • The previous logic would slice the file extension from the name. This would cause problems with any directory that contains a . in the name because that would be read as a file extension. The new logic using the path module would solve this issue because it only reads file extensions. It would return "" when a directory path is inputted.

The Pull Request

Once I was finished adding the feature in my forked repo, I was ready to create a PR to get my repo merged into the main project.

The PR: https://github.com/cychu42/staticSiteCon/pull/8

In the PR, I linked the original issue I created and mentioned that this PR resolves the issue. I gave an overview of the features added, further explained the features, and highlighted all the changed I made to the code. I tried to be as detailed as possible and give reasons for every change I made.

Once the PR was created, I waited for it to be reviewed and approved by the original author. They reviewed my changes and noticed that HTML files generated using the new startHtml function I created had whitespace issues. The raw HTML file would have tags heavily skewed to the right.

This was a simple fix and the author mentioned where they think the issue was occurring in a comment on the PR. I went ahead of fixed the spacing issue and pushed the changes to the branch. Once these changes were reviewed, they were accepted an merged in the main project.

Problems and Lessons Learned

I did not have much problems creating PRs and reviewing code as I had a bit of practice doing this previously.

However, one issue I encountered a bit too late was that I realized that Markdown syntax recognizes two bold tags: ** and __. My original PR only parsed ** in MD files. When I had realized that I missed the second syntax I had to create another issue and another PR to fix this problem.

I also learned that I need to commit and push often. I had realized too late that I made most changes under one commit. While it presented no issues during this PR, I know that's important to commit often whenever small issues have been addressed. So this is something that I will do more often in the future.

Pull Request on My Project.

I also received a pull request on my own repo.
The PR: https://github.com/alexsam29/ssg-cli-tool/pull/10

The PR on my project also added a feature to parser MD files and also added the ability to parse italicized text.

Review

I reviewed the changes made to my repo and found that the parser could not recognize special characters. So if word looked like this: *Sliver Blaze?*, then the word would not be italicized.

I requested changes on the PR to address the issue, which had something to do with the regex being used to parse italic text. They then fixed it and committed the changes. After reviewing and testing for a second time, I determined that the changes were fine and approved it to be merged. Overall, it was a very smooth process, there were no major hiccups.