Learning through Pull Request process as well as accepting others' PR
Published: 2023-09-23 03:56:18

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](https://github.com/rjwignar/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](https://github.com/rjwignar/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](https://github.com/rjwignar/ctil/issues/3) to inform the owner. 
![File an Issue](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/5few70m3844r1h2zqvom.png)

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](https://github.com/rjwignar/ctil/issues/3) page.
![assigned](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/693s5wdtc0i1ezy9ygki.png)

## 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](https://github.com/rjwignar/ctil) page. 
![Fork](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/o763003kfpyqnww3kzcf.png)
After click the fork button, the fork process is started, then the [forked project](https://github.com/mingming-ma/ctil) shows in my account.

![Forked](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/it6o7mxpaccccepmhf5z.png)

### 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](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/1608o3ujmrwgn4qke1ne.png)

### 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](https://github.com/mingming-ma/ctil/commits/issue-3).

### 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](https://en.cppreference.com/w/cpp/filesystem/path/extension) 

## 4. Create a Pull Request
When we finished all work, we need to push the working branch to our [forked repo](https://github.com/mingming-ma/ctil) and create a Pull Request. You can refer this [GitHub Docs](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request) 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](https://github.com/rjwignar/ctil/pull/4). 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](https://github.com/rjwignar/ctil/issues/3), GitHub will automatically link it to the Pull Request for you. Here I'm using the `#3` as shown on the [Pull request page](https://github.com/rjwignar/ctil/pull/4)
![PR content](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/m874rbh3ddvq7yvu1986.png)
And it linked in the [issue page](https://github.com/rjwignar/ctil/issues/3) as

![LinkedPR](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/qef7m73nykklckv40u1a.png)


## 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](https://github.com/rjwignar/ctil/pull/4).

![feedback](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/krb6c193q24rqijvsqsl.png)

Note that since I have already mark resolved the issues, it is now hided by default.
![hided](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/q2mzu4xw1ip7s8qz7cb0.png)

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](https://github.com/rjwignar/ctil/pull/4)
![Updated](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/0k4vn0hq2ycsn58lio8p.png)
You can see the commit [`678e15d`](https://github.com/rjwignar/ctil/pull/4/commits/678e15d7b56c828b5929ca87796dd133d03d635e) 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](https://github.com/rjwignar/ctil/pull/4)
![merged](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/89pdy3y0az73x7vqc9ej.png)


## 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](https://github.com/mingming-ma/txt2html/pull/8/files) :  **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](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/lmkepppf1budxfyv60h5.png)



![marks](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/c1y64c1l7fei3s92grcl.png)

Here are the options when a reviewer give all reviews
![options](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/803mbpvlo9y1ie7xnar9.png) 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.