Sending patches

[David Woodhouse]

If you're thinking of submitting fixes or improvements to get_iplayer
then first: thanks. It's great that you're helping out.

Even if the response you get is "OMG no! That's fugly!", it's still
useful. Even if the change which eventually gets committed looks nothing
like your own, your contribution was probably still worthwhile because
it helped to define the problem and a potential solution. (I'm saying
this here because I often forget to say it when I should.)

One thing that's *extremely* useful is if you can send patches in a form
which allows me (or whoever) to just save your mail directly as a file
and then 'apply' it to the source tree.

Firstly, that means using 'diff'. Rather than a prosaic description of
what you changed, please send a patch. That would look something like
this:

--- a/get_iplayer
+++ b/get_iplayer
@@ -5822,7 +5822,7 @@ sub get_stream_data_cdn {
 		# Common attributes
 		# swfurl = Default iPlayer swf version
 		my $conn = {
-			swfurl		=> "http://www.bbc.co.uk/emp/10player.swf?revision=18269_19216",
+			swfurl		=> "http://www.bbc.co.uk/emp/10player.swf?revision=18269_21576",
 			ext		=> $ext,
 			streamer	=> $streamer,
 			bitrate		=> $mattribs->{bitrate},

As well as being machine-readable, this is also human-readable. It tells
me which file to edit (ignore the leading a/ or b/), and roughly where
(it's showing me 7 lines at around line 5822). It shows me "context"
lines, so I know I'm looking in the right place. Usually three lines of
context either side of the changes. The line starting with a - sign
means "remove this line", and the line starting with + means "add this
line". It's quite simple, really.

If you've got get_iplayer from git using 'git clone', then after making
your changes you should be able to run 'git diff' to see the resulting
patch. Read it carefully and make sure you're only changing what you
*intended* to change. If you're making lots of other unrelated changes
(especially cosmetic changes like adding new empty lines or changing
whitespace), please fix that before sending the patch.

If you're not using git, you need to store a backup copy of the original
file before you modified it, and then you can use the 'diff' tool to
create a diff. Use the '-u' and '-p' options to make it give the most
useful type of output:
	diff -up get_iplayer.bak get_iplayer

Save the output to a file, and then send it TO YOURSELF in email (inline
as the body of the email, not as an attachment). Some email systems
misbehave and will word-wrap lines or convert tabs to spaces and do
other strange things. So send it to yourself first, save the resulting
email that you receive, and try to apply it with 'git am' or just with
'patch' to the original file. Don't send it to the list until that
works.

The tool we use to apply patches that we receive in email is called 'git
am', for 'apply mailbox'. As well as applying the patch itself, it also
picks up your name from the From: header, and the commit message which
explains what you changed and why. The first line of the commit message
comes from your Subject: header, and the rest from the top of your
email. A line containing just three dashes (---) marks the end of the
commit message. 

So if you have extra comments that don't want to go into the changelog,
like "this is the second version of the patch, changed to do A and B
like you asked. I didn't do C because I think you're being silly. Please
apply it anyway", then those can go *after* the --- so they aren't
preserved for posterity.

Please make sure that each patch does just one thing, and contains a
coherent commit message describing what it changes (and why, if that's
not obvious).

If you submit patches that can be applied directly with git-am, then
it's *easy* for us to apply them. If we have to edit things by hand
because the patches are mangled, or fix up a commit comment because you
didn't provide one, or stuff like that then it's usually going to wait
until we've got more time to do it. And that often means, in my case,
that I simply forget about it completely.

If your email system is *really* so broken that you can't include
patches as inline text in the email, then please include them as
text/plain attachments marked to be displayed inline so that they're at
least readable in the email. Make sure they start with a From: and
Subject: "header" line, and a reasonable commit message, so that they
can be saved and then applied with git-am, and that ought to suffice.

-- 
dwmw2