About Eyal Golan

Eyal is a professional software engineer and an architect. He is a developer and leader of highly sophisticated systems in different areas, such as networking, security, commerce and more

GIT Pull Requests Using GitHub

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

  1. 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.
  2. 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.
  3. 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)
  4. 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.
  5. 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.
  6. 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.
  7. 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”

  1. 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

    Mute Thread

  2.  

  3. 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’.
  4. 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.
  5. 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.

Compare & Pull Request

Compare & Pull Request

 

Assign Menu

Assign Menu

 

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.

Edit Diff Branch

Edit Diff 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.

Several Commits in Pull Request

Several Commits in Pull Request

 

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.

Branches can be automatically merged

Branches can be automatically merged

 

Confirm Merge

Confirm Merge

 

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

Post Merge / Closed Screen

Post Merge / Closed Screen

 

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>

Resources

https://help.github.com/articles/using-pull-requests
https://www.atlassian.com/git/workflows#!pull-request

Enjoy…

 

Related Whitepaper:

Software Architecture

This guide will introduce you to the world of Software Architecture!

This 162 page guide will cover topics within the field of software architecture including: software architecture as a solution balancing the concerns of different stakeholders, quality assurance, methods to describe and evaluate architectures, the influence of architecture on reuse, and the life cycle of a system and its architecture. This guide concludes with a comparison between the professions of software architect and software engineer.

Get it Now!  

Leave a Reply


× 1 = two



Java Code Geeks and all content copyright © 2010-2014, Exelixis Media Ltd | Terms of Use | Privacy Policy
All trademarks and registered trademarks appearing on Java Code Geeks are the property of their respective owners.
Java is a trademark or registered trademark of Oracle Corporation in the United States and other countries.
Java Code Geeks is not connected to Oracle Corporation and is not sponsored by Oracle Corporation.
Do you want to know how to develop your skillset and become a ...
Java Rockstar?

Subscribe to our newsletter to start Rocking right now!

To get you started we give you two of our best selling eBooks for FREE!

Get ready to Rock!
You can download the complementary eBooks using the links below:
Close