[libvirt] patch review tool for libvirt patches?

Laine Stump laine at laine.org
Fri Apr 8 14:35:05 UTC 2011


On 04/07/2011 02:21 PM, Daniel P. Berrange wrote:
> On Thu, Apr 07, 2011 at 11:24:04AM -0400, Laine Stump wrote:
>> Now that 0.9.0 is out, I'd like to ask everyone's opinions about
>> patch review tools.
>>
>> [...]
>>
>>
>> 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).
>>
>> [...]
>>
>> 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
>>
>> [...]
>>
>> Patchwork (http://ozlabs.org/~jk/projects/patchwork/) (used by Linux kernel&   kvm maintainers)
>>
> For me, any tool which requires visiting a web UI to submit or view
> patch code review comments/feedback is a non-starter. All code review
> feedback must be on the mailing list, and correctly threaded.


Agreed. That's why I made "optional" the first requirement. I wouldn't 
want anything that screwed up what's already working for somebody else, 
or made extra work for people who weren't interested, and also agree 
that having the mailing list archive available is very important 
(although I wish it was simpler to grab a message from the archive and 
"git am" it, or create a properly threaded reply to a message (for those 
times when I've already deleted a message from my mail client, then 
decide later that I want to do something with it)


> In other words, it would be a tool which serves a 'reporting' or
> 'tracking' patch series, not a code review management system.
> AFAICT, patchwork is the only one expressly designed in this
> manner. Their website sums it up nicely:
>
>    "patchwork should supplement mailing lists, not replace them


Well, that narrows down the field rather quickly :-)


>     Patchwork isn't intended to replace a community mailing list;
>     that's why you can't comment on a patch in patchwork. If this
>     were the case, then there would be two forums of discussion
>     on patches, which fragments the patch review process. Developers
>     who don't use patchwork would get left out of the discussion."


That seems a bit narrow-sighted of them. There's nothing in "all review 
must be on the mailing list" that would prevent a separate application 
from presenting the diff in some sort of UI (maybe even grabbing in the 
entire file so that things could be seen in context), allowing comments 
to be made, then generating email to the list that incorporates the 
comments. But that's a discussion for patchwork developers, not us...


> To also add to that, public mailing lists are a very good archival
> system for code review / discussions. Once in a mailing list, you
> can be pretty sure it'll never disappear from the web&  is always
> searchable from google. The same can't be said of most web apps.

Plaintext definitely has the most likelyhood of enduring, and of being 
usable by other applications.


So since the range of selection has been narrowed to 1 (well two 
actually - use patchwork, or don't use patchwork). Does anybody have 
experience setting it up and using it? Would it be simple for me to set 
it up locally and try it out.




More information about the libvir-list mailing list