Re: [libvirt] patch review tool for libvirt patches?
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.