GIT Pull Requests Using GitHub

Eyal GolanMay 14th, 2014Last Updated: May 13th, 2014

0 34 6 minutes read

Old Habits

We’ve been working with git for more than a year. The SCM was migrated from SVN, with all its history. Our habits were migrated as well. Our flow is (was) fairly simple: master branch is were we deploy our code from. When working on a feature, we create a feature branch. Several people can work on this branch. Some create private local branch. Some don’t. Code review is done one-on-one. One member asks another to join and walks through the code.

Introducing Pull Request to the Team

Recently I introduced to the team, with the help of a teammate the Pull Requests concept. It takes some time to grasp the methodology and see the benefits. However, I already start seeing improvements in collaboration, code quality and coding behaviors.

Benefits

Better collaboration
When a person does a change and calls for a pull request, the entire team can see the change. Everyone can comment and give remarks. Discuss changes before they are merged to the main branch.
Code ownership
Everyone knows about the change and anyone can check and comment. The result is that each one can “own” the code. It helps each team member to participate in coding and reviewing any piece of code.
Branches organization
There’s extra revision of the code before it is merged. Branches can be (IMHO should be) deleted after merging the feature. git history (the log) is clearer. (This one is totally dependent on the quality of comments)
Improved code quality
I see that it improves the code quality even before the code review. People don’t want to introduce bad code when knowing that everyone can watch it.
Better code review
We’ve been doing extensive code review since the beginning of the project. However, as I explained above, we did it one-on-one, which usually the writer explained the code to the reviewer. In my perspective, by doing that, we miss the advantages of code review. The quality of the code review is decreased when the writer explains the material to the reviewer. Using pull request, if the reviewer does not understand something, it means that perhaps the code is not clean enough. So more remarks and comments, thus, better code.
Mentoring
When a senior does code review to a junior, one-on-one, nobody else sees it. It’s more difficult for the senior to show case the expectations of how the code should look like and how code review should be performed. (there are of course other ways passing it, like code dojos. And, pair-programming, although it’s also one-on-one). By commenting review in the pull request, the team can see what’s important and how to review. Everyone benefits from review of other team members.
Improved git usage habits
When someone collaborates with the whole team, he/she will probably write better git comments. The commits will be smaller and more frequent, as no one wants to read huge amount of diff rows. So no one wants to “upset” the team. Using pull requests forces the usage of branches, which improves git history.

Objections

Others may call this section as disadvantages. But the way I see it, it’s more of complaints of “why do we need this? we’re good with how things were till now”

I get too many email already
Well, this is true. Using pull request, we start getting much more emails, which is annoying.
There’s too much noise. I might not notice important emails.
The answer for that is simple: If you are part of this feature, then this mail is important because it mentions code changes in some parts that you are working on. If you want to stop receiving emails for this particular pull request, you can ask to mute it.
Mute Thread

If we start emailing, we’ll stop talking to each other
I disagree with this statement. It will probably reduce the one-on-one review talks. But in my (short) experience, it improved our verbal discussions. The verbal discussion come after the reviewer watched the code change. If a reviewer did not understand something, only then she will approach the developer. The one-on-one discussions are much more efficient and ‘to the point’.
Ahh ! I need to think on better commit comments. Now I have more to think of
This is good, isn’t it? By using pull requests, each one of the team members need to improve the way comments are written in the commits. It will also improve git habits. In terms of smaller commits in shorter time.
It’s harder to understand. I prefer that the other developer will explain to me the intentions
Don’t we miss important advantages of code review by getting a walk though from the writer? I mean, if I need to have explanation of what the code does, then we better fix that code. So, if it’s hard to understand, I can write my comments until it improves.

How?

In this section I will explain briefly the way we chose to use pull requests. The screenshots are taken fron GitHub, although BitBucket supports it as well.

Branching From the “main” Branch

I did not write ‘master’ intentionally. Let’s say that I work on some feature in a branch called FEATURE_A (for me, this is the main branch). This branch was created from master. Let’s say that I need to implement some kind of sub feature in FEATURE_A. Example (extremely simple): Add toString to class Person. Then I will create a branch (locally out of FEATURE_A):

# On branch FEATURE_A, after pull from remote do:
# git checkout -b <branch-name-with-good-description>
git checkout -b FEATURE_A_add_toString_Person

# In order to push it to remote (GitHub), run this:
# git push -u origin <branch-name-with-good-description>
git push -u origin FEATURE_A_add_toString_Person
# Pushing the branch can be later

Doing a Pull Request

After some work on the branch, and pushing it to GitHub, I can ask for Pull Request. There are a few ways doing it. The one I find “coolest” is using a button/link in GitHub for calling pull request. When entering GitHub’s repository in the web, it shows a clickable notation for the last branch that I pushed to. After sending the pull request, all team members will receive an email. You can also assign a specific person to that pull request if you want him/her do the actual code review.

Changing the Branch for the diff

By default GitHub will ask to do pull request against master branch. As explained above, sometimes (usually?) we’ll want to diff/merge against some feature branch and not master. In the pull request dialog, you can select to which branch you want to compare your working branch.

Code Review and Discussion

Any pushed code will be added to the pull request. Any team member can add comment. You can add at the bottom of the discussion. And, a really nice option, add comment on specific line of code.

Merging and Deleting the Branch

After the discussion and more push code, everyone is satisfied and the code can be merged. GitHub will tell you whether your working branch can be merged to the main (diff) branch for that pull request. Sometimes the branches can’t be automatically merged. In that case, we’ll do a merge locally, fix conflicts (if any) and then push again. We try to remember doing it often, so usually GitHub will tell us that the branches can be automatically merged.

After the pull request is merged, it is automatically closed. If you are finished, you can delete the branch.

Who’s Responsible?

People asked

Who should merge?
Who should delete the branch?

We found out that it most sensible that the person who initiated the pull request would merge and delete. The merge will be only after the reviewer gave the OK.

Helpful git Commands

Here’s a list of helpful git commands we use.

# Automatically merge from one branch (from remote) to another
# On branch BRANCH_A and I want to merge any pushed change from BRANCH_B
git pull origin BRANCH_B

# show branches remotly
git remote show origin

# Verify which local branch, which is set to upstream can be deleted
git remote prune origin --dry-run

# Actual remove all tangled branches
git remote prune origin

# Delete the local branch
git branch -d <branch-name>