quick notes on using review board effectively

The Plasma team has been using review board for quite a while. We were the pioneering project within KDE for its use, in fact, which leads to an interesting observation about how such things often come up in KDE: one group tries something for a while, determines how well it works (or doesn't), other groups either start adopting it or express interest in it and eventually it is proven enough, consensus is reached and if it is a good thing it becomes a "KDE thing". It's a bit of a slow process at times, but it lets us as a community perform many experiments safely and pick out the ones that work well for people.

During our time using review board, I've noticed a few things that make it work better or worse, and I thought I'd share them here in case others were interested:

• Closing requests when they are done (committed, rejected, withdrawn, etc) is really important to keeping the queue useful. If I can't distinguish between reviews that have been languishing unattended for a month or a review that has been committed but not closed it isn't helpful.


• Screenshots for things that change the UI are hugely valuable. I've been tempted to start a "if it has a visual consequence and there is not screenshot, it won't be approved" policy to try and get more people doing this, but I also don't want to raise the bar too high to contribution. Still, screenshots make it so much faster and easier to communicate about a give patch.


• Give the patch a useful name before uploading it, otherwise I end up with a directory full of patches named "bug(N).diff" and other similarly unhelpful information. Names like "improve_foo_visual.diff" are much nicer. It makes it faster to apply them and lets me clean out my patches dir quicker if I can identify which is which.


• Start the diff at the lowest sensible directory. Diffing a change against a component 3 directories deep against the top level module isn't the most helpful: I tend to apply not at the top level but where the work is being done, e.g. at the application's or plugin's directory if in a combined module. For example, if patching the plasma-destop app, start the diff at kdebase/workspace/plasma/desktop/shell as opposed to kdebase or kdebase/workspace. This makes it easy to guess where the diff will start from and leave me at the "right" place in the tree once the diff is applied.


• Keep conversations on the review request. Usually, at least in KDE projects, all comments get CC'd to a mailing list. Replying to mailing list, however, results in the conversation being split up, partly on the review request and partly on the mailing list. Keeping it all on the review request keeps the comments together and closest to the actual data in question.

Do you have your own set of best practices for review board? Share them in the comments! I plan on adding such a list to Techbase for future reference. As review board is becoming a more and more common part of our workflow, using it efficiently is important.