[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]