Pull Requests and Patches with Git and Github

Overview of Pull Requests and Patches

A pull request is the differences between what the file looked like before your change and then after your changes, along with some explanatory text and a request for some project to incorporate the changes.  These are also known as patches or diffs.  The difference between a pull request and a patch is in how easy they are to produce and apply.  If approved, a pull request is generally a single click to pull in the changes.  To apply a patch, you generally need to run two commands, one to change the files in the working directory to the new version (known as applying the patch) and a second command to make a commit.  A generic term for either might be code change request, but often the term patch or pull request is used to mean either type of request.

Such code change requests are not only used when the submitter does not have write access to some repo.  Even with the ability to push to the master branch, if several developers try that at once, the repo can become corrupt!  The correct workflow is to submit your change for code review.  Once approved, the change can be merged into the master (or other) branch using the group's procedure for that.

“Pull request” is often considered a Github term, but it's really a term for any DVCS where you want to share a commit but don't have the access to push your changes.  So you then request someone with access to pull your commit.

Github Pull Requests

If you push your local repo to your GitHub account, you can use GitHub's GUI to do make pull requests easily.  Github calls a patch a “pull request”, and your request gets added to the parent repo in the Pull Requests tab for all to see and comment on.

To see that in action, create your own Github repo by making a fork of the class repo into your Github account.  (A “fork” is just a Git clone made on Github.)  Now import your copy into an Eclipse project.  Make a new branch, say “feature123” or “bugfix123” or anything.  Make a change and commit.

Push your new branch up to your Github repo.  Now go to Github, open your repo, and click make a pull request.  You make it between the master branch and your new branch.  The request will include the patch (all the diffs), and other info and goodies unique to GitHub.  You should explore all the information and features of a Github pull request.

Some of the nice features of a Github pull request is the they will test merge your changes and let you know if there's any conflict.  In the Pull Requests tab of a repo, you can view any pull request, read any comments made on it or add your own, and of course, view the actual diffs.  If you have write permission, you can accept the request and Github will automatically check out HEAD, apply your patch, and merge/commit the result, all with a single click.  If you prefer to review the changed code itself rather than the diffs, you can click a link to take you to the commit object, where you can view the files directly.

Making Github pull requests does require you to have your own branch or a clone of the repo on Github with your change.  If you don't have that, you have two options: make a code change request manually by making a patch and adding links and comments.  Or you can push your work onto your Github account area, and use that repo’s URL in the pull request.  This is known as fork-and-pull

(For our project #2, don't use Github's GUI to make a pull request.  Just view and copy your diffs from Github, or make a patch in Eclipse or another IDE.

Making Patches

If your repo isn't Internet accessible, nobody can pull from it.  So you need to post or email your request, which, just like a pull request, needs to include all the information needed to review and approve your request.  The diffs are in a standard format so they can be automatically applied.  This type of request is known as patch or patchset, so such requests are often called patches.  These are saved in files with the extension .pat or more commonly, .patch.

The standard format of the differences is called Unified diff, or sometimes patch format.  (See also using Gnu patch.)  If the change is approved, the patch can be automatically applied to the files that need to be modified.  However, the changes must then be committed into the repo.  Note that patches predate Git or any VCS.  Back then you used the Unix tool “diff” to produce patch files, and the tool “patch” to apply them.  Those tools are still used today, although most of us prefer using Git to produce and apply them, or some GUI tool.

Beyond the diffs, a patch or pull request contains additional information such as who made the changes, from where (which repo/branch/commit), any comments made by the submitter and subsequent code reviewers, and other stuff.

To see your diffs on Github (without making a Github pull request) is easy!  View a repo's history and open up any commit.  For example: 9bc4981fd1df2c8c2617165ec491ed27c7291774.  You can use the “unified” view to see the diffs in a way that is easy to copy and then save as the patch (in a file or email).  (The other view is “split” view, showing you the differences side-by-side.)

A pull request (or patch) might look similar to the following:

commit 9bc4981fd1df2c8c2617165ec491ed31c7291774
Author: Piffl, Hymie <hpiffl@hawkmail.hccfl.edu> 2018-02-06 00:14:22
Committer: Piffl, Hymie <hpiffl@hawkmail.hccfl.edu> 2018-02-06 00:14:22
Parent: 0cc721527fe3aedb951a787def5ea079f3c890b6 (Program update by adding name
and comment)
Branches: master, origin/master

My update

+
+       System.out.println( "Hi, My name is Hymie Piffl." );
+       counter++;

Notice in the above example that no lines were replaced; only three lines were added.  If your patch shows lots of deletions or replacements of lines you didn't think you changed, you probably did something wrong and should try again.

The exact format of a patch depends on what you used to create it (NetBeans, Visual Studio, Eclipse, TortoiseGit, plain old Git, or something else).  Here's one I created in Eclipse for another project, selecting the “git email header” format:

From 880abc6a96733de844c75c70169cf7b935041ca6 Thu, 15 Feb 2018 21:22:26 -0500
From: Wayne Pollock <pollock@acm.org>
Date: Thu, 13 Apr 2017 16:16:48 -0400
Subject: [PATCH] Finished all refactoring for issue #2.

Just needed to polish up the implementation of the simulator,
and implement the remaining test cases.

diff --git src/com/wpollock/bank/Account.java src/com/wpollock/bank/Account.java
index 1a79934..e831435 100644
--- src/com/wpollock/bank/Account.java
+++ src/com/wpollock/bank/Account.java
@@ -33,7 +33,8 @@
             throw new IllegalArgumentException(
                     String.format("Initial Balance must be >= zero (was %4.2f)"
                             + " for customer %s (acct desc: %s)",
-                            initialBalance, customer.toString(), accountDescription)
+                            initialBalance, customer.toString(),
+                            accountDescription)
                     );
         this.balance = initialBalance;
         if ( customer == null || accountDescription == null)

As you can see, the patch or pull request contains the differences between the old and new versions of any modified files, and some additional comments and info (such as the Git commit ID).  Patches do not depend on Git or Github, or even Java.  Rather, they are a standard way to share changes for code review purposes, and eventual inclusion in some project.

Standard patch format only shows differences between two files, which are referred to as file “a” and file “b”.  In the output, if the change is to a file between versions (the common case), the output shows the same file name for both.  When a file is added, deleted, renamed, or moved, the Git patch output will show additional information to let you see what's happening.  Adding a file shows as a diff between /dev/null and the new file; removing a file shows as a diff between the old file and /dev/null.  “/dev/null” is a special, always-empty file on Unix and Linux systems.  Finally, depending on the options chosen when making the diff output, Git may show a move or rename as a pair of add and remove diffs.

Here is a dummy patch I made in Git to show these features:

C:\Temp\practice>git diff -p master feature
diff --git a/LICENSE b/LICENSE.txt
similarity index 100%
rename from LICENSE
rename to LICENSE.txt
diff --git a/README.md b/README.md
index fc08d0d..80bf892 100755
--- a/README.md
+++ b/README.md
@@ -5,4 +5,5 @@ This is my first git repo on github.

 First edit.
 Second edit, made locally.
+last edit.
 another edit
diff --git a/dir/file.txt b/dir/file.txt
deleted file mode 100755
index dcfd1c8..0000000
--- a/dir/file.txt
+++ /dev/null
@@ -1 +0,0 @@
-bye
diff --git a/file.txt b/file.txt
new file mode 100755
index 0000000..c929649
--- /dev/null
+++ b/file.txt
@@ -0,0 +1 @@
+blah

To create a patch is easy in Eclipse, once you learn how.  First open your project in Eclipse.  Then:

  1. Right-click your project in the Eclipse Project Explorer, and select Team→Show in History.
  2. In the History window, right-click on the HEAD commit and select “Create Patch...”.  This creates the patch from the HEAD to the previous commit.  If you do this right after your commit, the HEAD is the commit with your changes.  If you waited and fetched (or pulled) changes from GitHub, your commit is no longer at HEAD.  In that case, right click on your commit to make the correct patch.
  3. Pick a location to save the patch in (I usually pick the desktop). Click the Next button.
  4. Change the format to “Git (one-line header)” or to “Git (e-mail header)”.  Click the Finish button.
  5. Done!  Open your file in an editor to see your diffs.  You can add text to that, to help the reader understand your code.  Then you email that to someone, hopping they will apply it and commit the change to their repo.  (Did you notice the “Apply patch...” menu item near the “Create Patch...” item?  If someone sends you a patch and if you approve the change, you can save their patch in a file and then easily apply it.)  With Github or other code repository servers, you can often upload your request for others to see and comment on.

How to Write the Perfect Code Change Request

(From github.com/blog)

  1. A proper code change request should include the purpose of the change, such as “This simplifies the display of...”, “This fixes handling of...”, or “This is an experiment to test a new feature that...”.
  2. A proper code change request should provide an overview of why the work is taking place (with any relevant links, such as to issues or tickets); don’t assume familiarity with the history.
  3. Code change requests should be as small as possible, focused on one change only, and be self-sufficient.  No developer will attempt to review a 5,000 line patch!  No change will be accepted if it depends on others (and thus, accepting your change would break the existing code).  Note it is not only allowed but encouraged to ask for guidance before writing some change request that may not be acceptable.
  4. Pull requests and patches can usually be read by anyone, not just the developer you hope reviews it.  So be careful what you say (don't disclose company secrets) and how you say it.
  5. Be explicit about what feedback you want, if any: a quick pair of eyes on the code; a discussion on the technical approach; a critique of the design or approach used; a compliance check for licenses, corporate policies, or security; etc.  Also be explicit about when you need the feedback by.
  6. If the code change request is work in progress, say so.  A prefix of “[WIP]” in the title of the request (the email subject) is a simple, common pattern to indicate that state.
  7. Mention individuals or other groups that you specifically want to involve in the discussion, and mention why.  It is common to use “@name” notation for this, for example “/cc @hymie for clarification on this logic” or “/cc @HCC/Security, do you have any concerns with this approach?”.

Here's a sample pull request comment I just happened to get in the mail the day I wrote this.  See if it you think it is a good request or needs improvement:

Hi,

A few weeks ago I wrote a patch that hacks OAuth support into
Davmail, allowing users whose IT departments require
MFA to continue using it.  It's very rough at the moment,
only allowing a single user at a time, and requiring some finicky
setup with refresh tokens etc, but is perfect for me.  I would be
happy to clean it up if there was interest in accepting it to mainline.

Patch is attached so you can get an idea for my hacky
implementation.  I have a few ideas as for how it can be cleaned up
to support concurrent users, gracefully informing the user how
they have to authorise, etc.

Cheers,

Would you spend time reviewing that patch?

Responding to Code Change Requests

When reviewing a pull request (or patch), you should familiarize yourself with the context of the issue before offering any feedback comments.  If you disagree strongly, consider giving it a few minutes before responding; think before you react.  Try hard to find a good reason for agreeing with the submitter.

Be polite.  For example, instead of “never do ...”, say “What do you think about trying...?”.

Never just agree or disagree.  Say why you think the request should be changed and resubmitted.  It is okay to disagree because of a personal preference, but say so.

If the patch is not acceptable, or even if it is but you think it could be improved, you should offer specific suggestions for that.  Instead of “This patch won't work well.”, you could say “I think you could simplify this by...” or “I think you could improve this by ...”.

Ask good questions (“What do you think about changing x to y?”; don't make demands (“This is okay, but change x to y first”).  Good questions avoid judgment and avoid assumptions about the author's perspective.

When you've finished your review and approve the code change request as is, sign off on it with a Ready to merge or similar comment.  (Some groups use a 👍 (“thumbs-up”) icon for this.)

Responding to Pull Request/Patch Feedback

Lead off with an expression of appreciation, even when feedback has been mixed.  The person(s) reviewing your request have put in a lot of time and effort, even if they don't agree with you.  Try to respond to every comment.

If you don't understand the feedback, ask for clarification.

If your code violates some group guideline or standard that you don't agree with, don't argue it here.  Instead, open up an issue to have the bad standard changed.  Meanwhile, make you code in compliance with existing standards.

If you do make a change as a result of feedback (or for any reason), your reply should link to any follow up commits or new pull requests.  (For example, “Thanks, that suggestion worked great!  Done in commit 1682851.”).

If the discussion doesn't seem to be going anywhere, see if there is another way forward.  Face-to-face meetings might be more effective than on-line discussions.  Any discussions that took place off-line should be summarized in a follow-up reply.  Or, take a day to remove your thoughts from this particular issue, then go back with a fresh prospective.  If you decide to drop the request, you should summarize the whole discussion for the sake of any readers to come across your request in the future, or for reference when a similar request is made later.

Keep in mind how difficult it can be to express emotion using on-line communication, and how easy it is to be misunderstood.  Always assume the people communicating with you have the best intentions.  Avoid language that can be mistaken for personal attacks.  If you cannot believe the comment was well meant, ask for clarification (politely), face-to-face if possible.  Also remember that any negative comments are about the code, and not about the person who wrote it.