Pull requests
A pull request, or PR for short, is a bunch of commits
that the person who created the PR would like to add to a repo.
Usually a PR fixes a bug, adds a new feature or cleans up messy code.
For example, in this PR,
I added a new feature to kanga333's comment-hider repo.
In most open-source projects, anyone can create a PR, and that's the recommended way to contribute to the project. For example, kanga333 never gave me any kind of permissions to create a PR. I just did it.
Making a PR¶
Usually you don't have the permissions to push a branch directly to the repo where the PR will go. To work around that, the first step is to fork the repo. It basically means making your own copy of the repo. For example, github.com/Akuli/comment-hider is my fork of github.com/kanga333/comment-hider.
To create your own fork repo, open the original repo on GitHub, and then click the "Fork" button in the top right corner:
Next you can clone your fork. Make sure you write your own username into the command; otherwise you clone the original repo, even though you don't have permissions to push there. (TODO: what to do if you already cloned the original repo?)
$ git clone https://github.com/username/reponame Cloning into 'reponame'... remote: Enumerating objects: 6, done. remote: Total 6 (delta 0), reused 0 (delta 0), pack-reused 6 Unpacking objects: 100% (6/6), done. $ cd reponame $ git lola * 9b66474 (HEAD -> main, origin/main, origin/HEAD) Merge branch 'multiplication' |\ | * 102c641 fix multiplication bug | * 2f8a77c multiplication code, not working yet * | d60059c fix subtraction bug |/ * 9e639ca create calculator.py * 7b47314 add better description to README * 50ec14e Initial commit
We will continue from this calculator example.
import sys first_number = int(sys.argv[1]) operation = sys.argv[2] second_number = int(sys.argv[3]) if operation == "+": print(first_number + second_number) elif operation == "-": print(first_number - second_number) elif operation == "*": print(first_number * second_number)
Next make a new branch. I will name it division.
$ git checkout -b division Switched to a new branch 'division'
Let's add a new feature to the calculator, and then commit and push it:
elif operation == '/': print(first_number / second_number)
$ git status On branch division Changes not staged for commit: (use "git add <file>..." to update what will be committed) (use "git restore <file>..." to discard changes in working directory) modified: calculator.py no changes added to commit (use "git add" and/or "git commit -a") $ git add calculator.py $ git diff --staged diff --git a/calculator.py b/calculator.py index c8f88eb..db81b3d 100644 --- a/calculator.py +++ b/calculator.py @@ -10,3 +10,5 @@ elif operation == "-": print(first_number - second_number) elif operation == "*": print(first_number * second_number) +elif operation == '/': + print(first_number / second_number) $ git commit -m "new division feature" [division 2194100] new division feature 1 file changed, 2 insertions(+) $ git push fatal: The current branch division has no upstream branch. To push the current branch and set the remote as upstream, use git push --set-upstream origin division $ git push --set-upstream origin division Username for 'https://github.com': username Password for 'https://username@github.com': Enumerating objects: 12, done. Counting objects: 100% (12/12), done. Delta compression using up to 2 threads Compressing objects: 100% (8/8), done. Writing objects: 100% (8/8), 5.76 KiB | 5.76 MiB/s, done. Total 8 (delta 5), reused 0 (delta 0) remote: Resolving deltas: 100% (5/5), completed with 4 local objects. remote: remote: Create a pull request for 'division' on GitHub by visiting: remote: https://github.com/username/reponame/pull/new/division remote: To https://github.com/username/reponame * [new branch] division -> division Branch 'division' set up to track remote branch 'division' from 'origin'.
Notice that the git push output includes a link.
Open it in your web browser.
Alternatively, you can open your fork or the original repo on GitHub,
and you should see a big and green "Compare & pull request" button:
Adjust the title and description of the PR as needed. Try to write the title so that it's easy to see what the PR does when it shows up in the same list with many other PRs.
Then click "Create pull request" and wait for someone to review the PR.
Reviewing¶
When your repo receives a PR, you shouldn't accept it blindly. Usually the PR needs more work, and you should leave feedback for the person who created it. This is called a code review, or a review for short.
First, go to the "Files changed" tab in the PR:
Read the changes. If there's something wrong with a specific line,
hover the line with your mouse, and a blue + icon should appear.
Click the blue + icon. This will let you comment the line.
If you want to do a specific change to the line, click the plus-minus button at top left corner.
The line should appear in the comment, and you can edit it to be like you wanted it.
For example, I changed ' to ", and briefly explained why.
When the comment is done, click "Start a review". Add more review comments similarly if needed, and then click "Finish your review" at top right. Fill in the dialog box that appears and then click "Submit review". Once done, your review should look something like this:
Responding to a review¶
Imagine you are the person who wrote the PR. When your PR has been reviewed, you need to fix any issues that the reviewer brought up. If they left a suggestion and you agree it's a good idea, just click "Commit suggestion", and then confirm by clicking the "Commit changes" button that comes up. If not, commit and push more changes to the branch, and the changes will automatically appear on GitHub.
The "Commit suggestion" button creates a commit only on GitHub.
On the other hand, git commit creates the commit only on your computer.
To see what happens when these collide, suppose you created a commit on your computer,
with the commit message commit created on cloned repo,
and you used the "Commit suggestion" button on GitHub.
When you try to push your commit, it fails:
$ git push To https://github.com/username/reponame ! [rejected] division -> division (fetch first) error: failed to push some refs to 'https://github.com/username/reponame' hint: Updates were rejected because the remote contains work that you do hint: not have locally. This is usually caused by another repository pushing hint: to the same ref. You may want to first integrate the remote changes hint: (e.g., 'git pull ...') before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details.
In this situation, as the error message suggests, you need to git pull before you can git push.
As you would expect, this downloads commits from GitHub to the cloned repo on your computer.
$ git pull remote: Enumerating objects: 5, done. remote: Counting objects: 100% (5/5), done. remote: Compressing objects: 100% (3/3), done. remote: Total 3 (delta 2), reused 0 (delta 0), pack-reused 0 Unpacking objects: 100% (3/3), done. From https://github.com/username/reponame 67a661d..a198d97 division -> origin/division Merge made by the 'recursive' strategy. calculator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) $ git lola * 73f3b05 (HEAD -> division) Merge branch 'division' of https://github.com/username/reponame into division |\ | * 1b9b4bd (origin/division) Update calculator.py * | 5f415a3 commit created on cloned repo |/ * 2194100 new division feature * 9b66474 (origin/main, origin/HEAD, main) Merge branch 'multiplication' |\ | * 102c641 fix multiplication bug | * 2f8a77c multiplication code, not working yet * | d60059c fix subtraction bug |/ * 9e639ca create calculator.py * 7b47314 add better description to README * 50ec14e Initial commit
Note that git pull created a merge commit;
it merges the commit on your computer (commit created on cloned repo)
and the commit created by the "Commit suggestion" button (Update calculator.py).
Therefore everything I wrote about
merges and merge conflicts applies here too.
As you can see from the git lola output, origin/division (i.e. the division branch on GitHub)
isn't up to date yet, and we need to push:
$ git push Enumerating objects: 2, done. Counting objects: 100% (2/2), done. Delta compression using up to 2 threads Compressing objects: 100% (2/2), done. Writing objects: 100% (2/2), 410 bytes | 410.00 KiB/s, done. Total 2 (delta 0), reused 0 (delta 0) To https://github.com/username/reponame a198d97..a7cf99d division -> division $ git lola * 73f3b05 (HEAD -> division, origin/division) Merge branch 'division' of https://github.com/username/reponame into division |\ | * 1b9b4bd Update calculator.py * | 5f415a3 commit created on cloned repo |/ * 2194100 new division feature * 9b66474 (origin/main, origin/HEAD, main) Merge branch 'multiplication' |\ | * 102c641 fix multiplication bug | * 2f8a77c multiplication code, not working yet * | d60059c fix subtraction bug |/ * 9e639ca create calculator.py * 7b47314 add better description to README * 50ec14e Initial commit
Merging a PR¶
Imagine you are the reviewer. After enough reviewing, when the PR looks good, click "Merge pull request". As you would expect, this merges the PR author's branch into your main branch.
Many people (including me) like to use "Squash and merge",
which causes the whole pull request to show up as just one commit in Git logs (e.g. git lola),
regardless of how many commits the PR author actually created.
You can find that option by clicking the little arrow in the merge button:
Making another PR¶
Imagine you are the person who wrote the division PR. While you are waiting for the division PR to get reviewed for the second time, and hopefully merged, you look at the code and you notice something: it doesn't handle invalid inputs for the operation. There's no error, and it just doesn't do anything.
$ python3 calculator.py 1 + 2 3 $ python3 calculator.py 1 / 2 0.5 $ python3 calculator.py 1 lolwut 2
git checkout -b, there are a couple gotchas to be aware of:
-
Your new PR should be based on what's on the
mainbranch, not based on what you have on thedivisionbranch. Otherwise the new PR will also contain the division code, which isn't nice for the reviewers; they want to review one thing at a time. Becausegit checkout -bmakes the new branch based on your current branch, you can fix this by first switching to themainbranch. -
Maybe it's been a while since you cloned the repo, and since then, the original repo has gotten other commits and PRs. To avoid unnecessary merge conflicts, your new PR should be based on the latest commit of the original repo. Pull from the original repo to fix this.
For this illustration, suppose that the original repo has gotten two new commits unrelated to your PRs,
with commit messages unrelated commit 1 and unrelated commit 2.
Let's fix the problems:
$ git lola * 73f3b05 (HEAD -> division, origin/division) Merge branch 'division' of https://github.com/username/reponame into division |\ | * 1b9b4bd Update calculator.py * | 5f415a3 commit created on cloned repo |/ * 2194100 new division feature * 9b66474 (origin/main, origin/HEAD, main) Merge branch 'multiplication' |\ | * 102c641 fix multiplication bug | * 2f8a77c multiplication code, not working yet * | d60059c fix subtraction bug |/ * 9e639ca create calculator.py * 7b47314 add better description to README * 50ec14e Initial commit $ git checkout main Switched to branch 'main' Your branch is up to date with 'origin/main'. $ git pull https://github.com/where_you_forked_it_from/reponame remote: Enumerating objects: 2, done. remote: Counting objects: 100% (2/2), done. remote: Total 2 (delta 1), reused 2 (delta 1), pack-reused 0 Unpacking objects: 100% (2/2), done. From https://github.com/where_you_forked_it_from/reponame * branch HEAD -> FETCH_HEAD Updating 9b66474..9a6cf94 Fast-forward $ git lola * 9a6cf94 (HEAD -> main) unrelated commit 2 * 38b8add unrelated commit 1 | * 73f3b05 (origin/division, division) Merge branch 'division' of https://github.com/username/reponame into division | |\ | | * 1b9b4bd Update calculator.py | * | 5f415a3 commit created on cloned repo | |/ | * 2194100 new division feature |/ * 9b66474 (origin/main, origin/HEAD) Merge branch 'multiplication' |\ | * 102c641 fix multiplication bug | * 2f8a77c multiplication code, not working yet * | d60059c fix subtraction bug |/ * 9e639ca create calculator.py * 7b47314 add better description to README * 50ec14e Initial commit
With this pull, make sure to specify the original repo, not your fork; if you find yourself writing your own GitHub username into the command, you are likely doing it wrong.
You might notice that origin/main is left behind, even though the two unrelated commits are on GitHub.
This is because origin/main means your fork's main branch, as that is what you cloned.
You can run git push on the main branch if you want,
but most people don't bother with updating the main branches of their forks.
To create a new PR from here, create a new branch and so on as described above.
Conflicts in a PR¶
As explained above, the original repo can change while your PR isn't merged yet. If it changes in a way that conflicts with your PR, you will see this in GitHub:
To solve this, pull from the original repo as above, but do it on your PR branch. You will get merge conflicts that you will have to resolve.
For example, if the division PR conflicts, you would do this:
$ git checkout division Switched to branch 'division' Your branch is up to date with 'origin/division'. $ git pull https://github.com/where_you_forked_it_from/reponame remote: Enumerating objects: 2, done. remote: Counting objects: 100% (2/2), done. remote: Total 2 (delta 1), reused 2 (delta 1), pack-reused 0 Unpacking objects: 100% (2/2), done. From https://github.com/where_you_forked_it_from/reponame * branch HEAD -> FETCH_HEAD Auto-merging calculator.py CONFLICT (content): Merge conflict in calculator.py Automatic merge failed; fix conflicts and then commit the result.