Learning through Pull Request process as well as accepting others' PR

mingming-ma

Mingming Ma

Posted on September 23, 2023

Learning through Pull Request process as well as accepting others' PR

Open-source communities require teamwork. If you want to contribute code to others, you need to submit a Pull Request (PR). This blog post provides a detailed process, from starting with proposing an issue to having your code eventually accepted (merged) by others. I'm going to explain by using my PR on this project ctil as an example. I'll also share some of my mistakes and what I learned in the process. Hope it helps!

A Brief Background of the Project and my PR Motivation

This project ctil is a command-line tool written in C++ that processes .txt files into .html files. I was planning to contribute a feature to this tool, allowing it to read Markdown(.md) files as inputs and to understand the Italics grammar (*word* or _word_ is meaning word). As a result, the generated HTML files can display correct Italics fonts when opened in a web browser.

1. Inform the owner: File an Issue

First thing I need to let the owner of the project know I'm going to contribute, usually we can file an issue to inform the owner.
File an Issue

Because the pr is already merged, so the status now shows as Closed.

When the owner got notification and accepted, I was assigned to this issue. It is shown right of the issue page.
assigned

2. Prepare the code: Fork, Clone, Branch

Fork

Next step is get the code ready. We need to fork the original project so we can have a downstream project. You can find the fork button on the target project page.
Fork
After click the fork button, the fork process is started, then the forked project shows in my account.

Forked

Clone

Now we can clone the code to our local machine to develop new features. I use this command git clone [https-url]. You can get the https-url by clicking the code button.
clone

Branch

Next, it is always a good idea to have new branch when we develop a new feature. In my example, I choose issue-3 as this is the 3rd issue of this feature.

3. Write the code:

When the code is prepared, we can write the code. Here you can see my commits.

Some details about how I implemented if you interested

  • Because C++ not have a direct way to check the suffix, I wrote a new helper function bool ends_with(std::string const & value, std::string const & ending)
  • I use a quick and easy approach to do the two different content handling content convert logic:
    • Keep old generateHTML function logic and update its name to generateHTML_txt so that only process text files
    • have a new generateHTML_md function that only process Markdown files
  • md_italics_content_update helper function that only check italics grammar and convert the string adding needed HTML tag.

Update:
There IS actually a direct extension check in C++, see std::filesystem::path::extension

4. Create a Pull Request

When we finished all work, we need to push the working branch to our forked repo and create a Pull Request. You can refer this GitHub Docs link to do that. I used the Web browser way and it directly shows you which branch you want to merge to the target repo. You can see how I wrote the pull request content. It's a good practice to note which issue your PR aiming to solve when we fill the content, typing # can give all your opened issues number with the title. After you chose that number, on the issue page, GitHub will automatically link it to the Pull Request for you. Here I'm using the #3 as shown on the Pull request page
PR content
And it linked in the issue page as

LinkedPR

5. Update the Pull Request based on the feedback

After you fill the PR content and sent the PR to the owner, the owner will let someone review your contribution and give feedback.

The feedback will show on the Pull request page.

feedback

Note that since I have already mark resolved the issues, it is now hided by default.
hided

We need update code at same branch and again push to our forked repo, and GitHub will update the Pull Request commits on the Pull requests page
Updated
You can see the commit 678e15d is after the reviewer sent the requested changes which means GitHub updated the Pull Request commits to let the reviewer can do a second review.

If the reviewer approve your PR, you can see the merged status on the Pull requests page
merged

6. What need to do if you get PR from others

As you can see from above, the review is the main process to accept other's PR. To review the changes, the reviewer check on the Pull requests page : Files changed section. When the cursor is hanging on each line of the code, the blue plus button will show and the reviewer can leave the comments behind.
changes

marks

Here are the options when a reviewer give all reviews
options If the PR is accepted, the reviewer can check the second option to merge the PR. If not, the reviewer can leave comment only (the first option) or inform for waiting updated PR (the third option).

7. Mistakes and lessons

  • I made a mistake that I neglected the _word_ is also italics grammar. I didn't realize there are actually two methods to show Italics.

    • learned lesson: should be more careful on the spec next time.
  • I had error that I couldn't push to my forked repo,
    fatal: The current branch master has no upstream branch

    • learned lesson: git remote url is not correct, check git clone method.
💖 💪 🙅 🚩
mingming-ma
Mingming Ma

Posted on September 23, 2023

Join Our Newsletter. No Spam, Only the good stuff.

Sign up to receive the latest update from our blog.

Related