Saturday, February 23, 2008

On patch review

Improving the patch review process was one of the much discussed topics for the 8.4 release cycle. The recurring commit fests are one idea that will be tried out. Here is what I think. If you want more, better, and earlier review happening, you need to facilitate reviewing in many ways.
 
 We currently have a well-established and arguably successful process for maintaining existing features. We have a public source code repository, test infrastructure, and many people willing and able to contribute. Committers can fix small issues immediately and larger issues with small overhead. Others can work on small issues with the small overhead of sending a patch that will soon get committed, and they can work on large issues, well, that is the debate. If more than one person wants to contribute, they obviously can. We have many hands and eyes working on everything, and that works well.
 
 We don't have any of that for new feature patches, the staging area of development. The source code repository is a set of possibly related multiple 10k patches spread around the mailing list archives. The only way for others to get involved in small or large ways is by chance learning about the feature proposal, fetching the patch, which will usually no longer apply cleanly, alter it and send an equally huge patch back. If more than one person does that, it's impossible, unless you want to get involved in interdiff madness. Meanwhile the original developer has to sit still waiting for review, or more likely, continues development, which invalidates all the review work.
 
 Now we also know that reviewing the core logic of patches is in fact hard and needs skilled people of which we would always like to have more. But much of our CVS-based development works with many little hands, and so should feature patch review.
 
 Here is how I think patch review could work better. Someone writing a feature patch should use a distributed version control system that ties into our currently-CVS trunk. He publishes that repository either via web or via email. Now the little hands can get involved immediately. They can pull local copies and review the commit history. They can send small bug fixes, formatting, wording improvements. They can update the patch for new developments in the mainline or investigate collisions with other features in development. Someone else can start writing documentation. The original developer can merge those changes back and continue based on them. Meanwhile, a useful revision history is always available for more people to get involved. Additionally, by a simple link published in a wiki, say, you have obsoleted all the needs for patch trackers or the like. Other people, perhaps more skilled reviewers can offer early comments and easily review development progress.
 
 That is how I would like to work. I have started to use the Git repository for my development work now because I believe that could make it happen. But you can replace your own technology if you think it can achieve similar effects. (In fact, Git is in my mind so ridiculously better than our current anonymous CVS setup and replaces CVSup as well, there is no reason -- other than the need to learn some new tools -- for anyone who has no CVS write access to even bother with CVS anymore.)
 
 Incidentally, quite a few people have shown interest during the mailing list discussion in trying out source code repositories mirrored off CVS to something of their liking. I hope that way we can find more optimal development methods in the near future.