What makes a good patch

First, what is a patch?

A patch is a code change. If you have the source to task, and you make changes, you can generate a diff. Here's a transcript of a trivial change being made, and a patch being generated:

% git clone git://tasktools.org/task.git task.git
...
% cd task.git
% echo 'Hi there' >> README
% git commit -a -m "Added a line to README" 
% git format-patch HEAD^
0001-Added-a-line-to-README.patch
%

You can see that one line was added to the README file, and the patch is correspondingly small.

% cat 0001-Added-a-line-to-README.patch
From 594db6eee470fc81f10a9fe2970d2333ea27db1f Mon Sep 17 00:00:00 2001
From: John Doe <john@doe.org>
Date: Mon, 1 Jun 2009 19:29:26 -0400
Subject: [PATCH] Added a line to README

---
 README |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/README b/README
index 39aa569..aaf8924 100644
--- a/README
+++ b/README
@@ -53,3 +53,4 @@ Got an idea for an enhancement?  Post a message!
 I have found that task makes me more productive and organized.
 I hope task can do the same for you.  

+Hi there
-- 
1.6.3.1

The next step would be to mail in the patch to . But there's more...

How could that be better?

Suppose the patch fixes some problem you have noticed. You have already done your part to contribute to task. Well done and thank you. But is there more?

Yes, there is more. Here are a few things that could make the difference between a patch and a great patch:

  • Does any of the associated task documentation also need to be updated?
  • Is there a unit test you could add that passes now, but would have failed before the application of the patch? This would be a great unit test that helps prevent future regressions.
  • Are there perhaps more unit tests that cover a few more corner cases and quirks? The more the better.
  • Does the change you made fit in with the existing code style? It should not look as if two people edited the file. That way we preserve consistency, which is important in a code base.

Why might a patch be rejected?

A patch submission is a generous gift, and is treated with the respect that deserves. All patch submissions are credited in the AUTHORS file. But there are reasons why a patch may be rejected. They are:

  • The patch doesn't cleanly apply. This could mean that the patch was made on the wrong branch, or many changes have occurred to the same file(s) since then.
  • The patch is complex, and no unit tests were provided.
  • The patch fixes a personal preference, and not an actual bug. The reviewer of the patch may agree with the patch, or may not.
  • The patch causes other issues to come to the surface.
  • The patch doesn't mesh with existing plans. Perhaps plans are in place that conflict, or solve the problem in a different way.
  • The patch fixes something that is already fixed.

Whenever a patch is rejected, the reasons for the rejection are included, and are intended to guide the author toward making a great patch.

Also available in: HTML TXT