The general concept sounds good, but there's a few things I'd like to see and want to make sure of.

1) Since we're now doing diffing ourselves, we can avoid creating those empty files. There's no point anymore, as we're just creating them, reading them in, and then deleting them. So we can just skip all that and pass an empty buffer to unified_diff().

2) This is sort of minor right now, but we're losing the ability to list the C function in the file. Review Board isn't doing anything with this yet, but it's nice when actually pulling down the diff. Probably okay not to have it though, I suppose.

3) We're doing our own diff but expecting data from GNU diff. Does unified_diff actually generate "Files ... and ... differ" or "Binary files ... and ... differ" ? If not, we need to find a solution for this.

4) A lot of that post-processing should probably go away. We're replacing the date stamps with revisions, yet we're supplying those initial date stamps anyway. Seems we should just skip a lot of this and provide our own revision.

5) Ideally this would be expanded to be used for other tools. Right now, we have tools (svn in particular) that calls out to GNU diff for the actual diffing. It would be nice if we could call out to post-review with a special argument that did all the diffing work so we wouldn't need an external diff tool. That may be a later thing though.

These should be in the list alphabetically.

These will leak. You need to call close on the file handle.

This needs to wrap to ~75 characters.

Should wrap to ~75 characters.