Reviewing and applying a patch

I've been fortunate enough to have been reviewing a lot of patches recently. Fortunate, because it means other people are contributing to my library. However, few of these contributions are without their problems; as with all contributions, each patch generally goes through two or three iterations before I think it's near enough to being ready that it's easier for me to apply the patch than it is to comment on it and request an updated version.

Generally, when patches get to this stage, it's just the really small, nitpicky things which are still wrong. Rogue whitespace, quirky indentation, typos in documentation…these are all really minor things, but they take time to check through and correct. One of the latest libgdata patches was, I think, a bit of a rushed job; and so there were more of these niggly problems than usual. Instead of going through and fixing all the problems and never giving detailed feedback about them to the patch contributor – as is usually the way, since the changes are so trivial, albeit cumulatively not inconsiderate – I decided to make a screencast of the process I go through when reviewing a patch.

Reviewing and applying a patch

It's about half an hour long, unscripted and unedited, so there are a couple of mistakes and omissions in there (for example, I later noticed I'd forgotten to mention the lack of input validation on the new function). However, I think it's quite comprehensive. It's aimed at those who are getting used to the open-source patch submission and development process, though those more talented than me welcome to watch it and lambast me for not using Emacs. Hopefully it's useful to someone, anyway. It's licenced under cc-by-sa 2.0.

6 thoughts on “Reviewing and applying a patch

  1. Alberto Ruiz

    Nice! Very useful piece of documentation, will watch it tomorrow from the office's adsl, you should poke the marketing list about it to find a right place to place it!

  2. Richard

    Thanks for making this.

    I'm generally filled with a lot of anxiety as I review the patch I'm about to submit, wondering which issues I'm missing 🙂

  3. blankthemuffin

    Interesting to watch. It's easy to underestimate the work that goes on behind the scenes. It's good to throw it out into the wild every now and then. 😀

  4. space cowboy

    For hunting spaces with gedit enable plugin "Draw Spaces" and toggle "View -> Show White Space"

    1. Philip Withnall Post author

      I have that enabled on my other computer, and it's quite useful, but perhaps not right for this job; searching for extraneous whitespace with it in this case would involve lots of scrolling and peering at the end of lines.

Comments are closed.