[libvirt] patch review tool for libvirt patches?

Laine Stump laine at laine.org
Thu Apr 7 15:24:04 UTC 2011


Now that 0.9.0 is out, I'd like to ask everyone's opinions about patch 
review tools.

I've been noticing lately that the volume of libvirt patches has 
increased significantly, and it's getting more and more difficult for me 
to keep up with reading the traffic, much less doing my part to review 
the patches. It would be very helpful if I could just see a view of 
pending patches, each properly colorcoded (or, even better, hidden) 
based on a patch being ACKed, deprecated, someone else in process of 
reviewing, etc. I know there are a few tools available, but I've never 
used any of them and wonder if anyone else has, and if they might be 
able to make a recommendation on something the libvirt project could use 
(or maybe something I could just use myself by sending a list feed into 
an application).

I'm sending this message 1) to see if others are feeling the pressure of 
the extra traffic too (or is my brain just processing more slowly :-/), 
and 2) to learn what your opinions are of setting up such a system for 
libvirt (for *optional* use only), and any opinions you have on what's 
available (or maybe you could provide a recipe for how you already 
manage all the patches without going crazy).

Here's my list of requirements; feel free to add/shoot down:

1) usage must be optional, so it must be able to update itself via 
monitoring messages on the list (a minimal amount of change/addition to 
the current message flow might be acceptable, but nothing major, and 
encountering PATCH/review/ACK messages as currently sent shouldn't make 
it blow up).

2) keep track of patches that are posted, NACKed, ACKed, re-posted 
(deprecating the original), and pushed (this should be done by 
monitoring git, not by looking at emails, as I've noticed patches are 
often pushed without a corresponding message to the list).

3) would also be nice if there was a place in the UI that those wanting 
to could "take" a patch for review so that two people didn't spend a lot 
of time on something not requiring that much attention, at the expense 
of other ignored patches.

4) maintain patchsets according to the "n/n" notation in the header (I 
mention this because one comment I saw about Gerrit said that it didn't 
do this).

5) provide some sort of interface for viewing the patch, annotating it 
with comments, and sending that back to the list as an ACK/NACK or 
simply a comment.

6) provide a simple way to save a patchset to files that can be "git 
am"ed (or maybe even do it for you, automatically creating a branch 
first, etc. This could possibly even be extended into a command to push 
a given patchset)

7) (of course it must be open source. Do I even need to say that? :-))

Ideally, a person wanting to use this system should be able to setup 
their email to filter all libvir-list traffic containing "PATCH" in the 
subject line, then create an account on the patch review system and 
handle all patch review via the tool's interface

Here's a list of tool that Rich Jones came up with in another discussion 
on the same topic, along with a few others that were mentioned in the 
ensuing discussion. To the right, I've included comments from others 
that seemed interesting to me. Please point out any that you disagree with!

Gerrit (https://code.google.com/p/gerrit) "The assumption that one issue == one patch is too deeply embedded"

Kiln (http://www.fogcreek.com/kiln/) (proprietary, so probably shouldn't be considered)

Patchwork (http://ozlabs.org/~jk/projects/patchwork/) (used by Linux kernel&  kvm maintainers)


http://www.reviewboard.org/  "didn't work well with git"


Also someone pointed out that gitorious has code review aids:

http://blog.gitorious.org/2009/11/06/awesome-code-review/


If I were going to investigate one of these and try setting it up, which do you think would have the greatest likelyhood of success (if any)?




More information about the libvir-list mailing list