* [Qemu-devel] Fwd: Proposal: Improving patch tracking and review using Rietveld
@ 2011-01-27 7:55 Paolo Bonzini
2011-01-27 10:19 ` Stefan Hajnoczi
2011-01-27 17:31 ` [Qemu-devel] " Anthony Liguori
0 siblings, 2 replies; 9+ messages in thread
From: Paolo Bonzini @ 2011-01-27 7:55 UTC (permalink / raw)
To: qemu-devel
Forwarding this from the GCC mailing list. Since patchwork isn't more
than a mail archive the way it's implemented in QEMU, this may be a more
interesting possibility.
Paolo
> At Google we use a code review tool which was open sourced a couple of
> years ago: Rietveld
> (http://code.google.com/appengine/articles/rietveld.html).
>
> The best way of thinking about it is "bugzilla for patches". The
> system creates an entry for every patch submitted, provides a web tool
> for manipulating the patch (comments, different views of the diff,
> highlighting, etc) and it also has an email gateway.
>
> We have discussed patch tracking mechanisms in the past, and none so
> far has taken hold. The reason why I like Rietveld is that it doesn't
> really matter whether we all switch to using it at once:
>
> 1- Rietveld always send the patch sent to it to gcc-patches@ (provided
> the submitter added gcc-patches to the CC list).
> 2- The whole trail of discussion on the patch also get sent to
> gcc-patches and everyone else is CC'd in it.
> 3- Reviewers do not need to use the web tool to reply to the patch.
> One can simply respond to the e-mail, and it will get added to the
> patch discussion trail.
>
> So, for people who do not want to use the tool, Rietveld will not get
> in the way. They can simply respond to the patch as usual, and as
> long as they keep the rietveld email address in the CC list, the patch
> trail will be updated automatically.
>
> At Google we will start using Rietveld to send patches. The only
> difference folks will notice is that Rietveld-generated email has some
> extra text.
>
> I have created a wiki page that explains the basics of using Rietveld
> (thanks Jeffrey for the instructions):
> http://gcc.gnu.org/wiki/rietveld
>
> Once again, I'd like to underscore the fact that if a patch submitter
> chooses to use Rietveld for tracking their patches, this should not
> affect in any way the traditional mail-based review. All I ask is
> that reviewers maintain the CC and Subject line intact in order to not
> confuse the tool.
>
> Jeffrey, would you mind looking over the instructions I've written to
> make sure they're correct?
>
> Richard, this is the tool I mentioned in today's chat.
>
>
> Thanks. Diego.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] Fwd: Proposal: Improving patch tracking and review using Rietveld
2011-01-27 7:55 [Qemu-devel] Fwd: Proposal: Improving patch tracking and review using Rietveld Paolo Bonzini
@ 2011-01-27 10:19 ` Stefan Hajnoczi
2011-01-27 10:23 ` Paolo Bonzini
2011-01-27 17:31 ` [Qemu-devel] " Anthony Liguori
1 sibling, 1 reply; 9+ messages in thread
From: Stefan Hajnoczi @ 2011-01-27 10:19 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On Thu, Jan 27, 2011 at 08:55:30AM +0100, Paolo Bonzini wrote:
> Forwarding this from the GCC mailing list. Since patchwork isn't
> more than a mail archive the way it's implemented in QEMU, this may
> be a more interesting possibility.
What features are you looking for beyond archiving?
It would be nice to have a dashboard of currently unapplied patches
(with old versions of patch series ignored). There should be an
automatic way of letting the review system know which patches have been
applied to the code repository.
BTW the email integration sounds good and is critical. If we have an
isolated system that requires separate log in and manual syncing of
status between mailing list, code repository, bug tracker, and review
tool then it will probably just wither away.
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] Fwd: Proposal: Improving patch tracking and review using Rietveld
2011-01-27 10:19 ` Stefan Hajnoczi
@ 2011-01-27 10:23 ` Paolo Bonzini
2011-01-27 10:34 ` Stefan Hajnoczi
2011-01-27 16:26 ` [Qemu-devel] " Paolo Bonzini
0 siblings, 2 replies; 9+ messages in thread
From: Paolo Bonzini @ 2011-01-27 10:23 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel
On 01/27/2011 11:19 AM, Stefan Hajnoczi wrote:
> On Thu, Jan 27, 2011 at 08:55:30AM +0100, Paolo Bonzini wrote:
>> Forwarding this from the GCC mailing list. Since patchwork isn't
>> more than a mail archive the way it's implemented in QEMU, this may
>> be a more interesting possibility.
>
> What features are you looking for beyond archiving?
Well, you pretty much nailed it.
> It would be nice to have a dashboard of currently unapplied patches
> (with old versions of patch series ignored).
and this:
> BTW the email integration sounds good and is critical.
:)
The only tricky point is whether email integration includes ignoring
older versions of patch series. I'll ask on gcc@gcc.gnu.org how/whether
that works.
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] Fwd: Proposal: Improving patch tracking and review using Rietveld
2011-01-27 10:23 ` Paolo Bonzini
@ 2011-01-27 10:34 ` Stefan Hajnoczi
2011-01-27 16:26 ` [Qemu-devel] " Paolo Bonzini
1 sibling, 0 replies; 9+ messages in thread
From: Stefan Hajnoczi @ 2011-01-27 10:34 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On Thu, Jan 27, 2011 at 10:23 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 01/27/2011 11:19 AM, Stefan Hajnoczi wrote:
>>
>> On Thu, Jan 27, 2011 at 08:55:30AM +0100, Paolo Bonzini wrote:
>>>
>>> Forwarding this from the GCC mailing list. Since patchwork isn't
>>> more than a mail archive the way it's implemented in QEMU, this may
>>> be a more interesting possibility.
>>
>> What features are you looking for beyond archiving?
>
> Well, you pretty much nailed it.
Glad we're on the same page. I wanted to see if you are looking for
code review or patch tracking.
I actually like the code review process used today - inline patches to
the mailing list. It's hard to add value to that without making it
more cumbersome.
But patch tracking is definitely lacking and would benefit from
something that aggregates and presents an accurate view of the state
of unapplied patches.
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] Re: Fwd: Proposal: Improving patch tracking and review using Rietveld
2011-01-27 10:23 ` Paolo Bonzini
2011-01-27 10:34 ` Stefan Hajnoczi
@ 2011-01-27 16:26 ` Paolo Bonzini
2011-01-27 16:31 ` Diego Novillo
1 sibling, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2011-01-27 16:26 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Stefan Hajnoczi, Diego Novillo, qemu-devel
On 01/27/2011 11:23 AM, Paolo Bonzini wrote:
> On 01/27/2011 11:19 AM, Stefan Hajnoczi wrote:
>> On Thu, Jan 27, 2011 at 08:55:30AM +0100, Paolo Bonzini wrote:
>>> Forwarding this from the GCC mailing list. Since patchwork isn't
>>> more than a mail archive the way it's implemented in QEMU, this may
>>> be a more interesting possibility.
>>
>> What features are you looking for beyond archiving?
>
> Well, you pretty much nailed it.
>
>> It would be nice to have a dashboard of currently unapplied patches
>> (with old versions of patch series ignored).
>
> and this:
>
>> BTW the email integration sounds good and is critical.
>
> :)
>
> The only tricky point is whether email integration includes ignoring
> older versions of patch series. I'll ask on gcc@gcc.gnu.org how/whether
> that works.
Here's the outcome of my conversation with the GCC developer who
proposed it.
- the tool would require adoption of a special tool for patch submitter.
The tool is a self-contained Python script that could be included in
the QEMU repository (upload.py).
- reviewers (and submitters discussing the issue) can either reply
directly to email, or use the web tool. Diego said that either way can
be used, but the web tool is actually pretty addictive. It produces
basically the equivalent of the "inline comment" mails we use, and has
decent keyboard bindings (that said, creating a comment always requires
a double click).
That said, it looks like the integration with git is (still?) a bit too
rough to be usable. In particular, you can more or less track a patch
series but not the commit messages of each series.
I created two issues in the tracker as examples:
- http://code.google.com/p/rietveld/issues/detail?id=267
- http://code.google.com/p/rietveld/issues/detail?id=268
http://code.google.com/p/rietveld/issues/detail?id=262 is also of
interest, even though it refers to Mercurial.
It's possible that if these are fixed, the remaining problems can be
worked around by hacking upload.py or wrapping a custom script around
it, that would be more similar to git-send-email in appearance and behavior.
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] Re: Fwd: Proposal: Improving patch tracking and review using Rietveld
2011-01-27 16:26 ` [Qemu-devel] " Paolo Bonzini
@ 2011-01-27 16:31 ` Diego Novillo
0 siblings, 0 replies; 9+ messages in thread
From: Diego Novillo @ 2011-01-27 16:31 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Stefan Hajnoczi, Paolo Bonzini, qemu-devel
On Thu, Jan 27, 2011 at 11:26, Paolo Bonzini <pbonzini@redhat.com> wrote:
> - reviewers (and submitters discussing the issue) can either reply directly
> to email, or use the web tool. Diego said that either way can be used, but
> the web tool is actually pretty addictive. It produces basically the
> equivalent of the "inline comment" mails we use, and has decent keyboard
> bindings (that said, creating a comment always requires a double click).
Right.
Given that you folks are just starting to look for code review tools,
you may want to consider other options we have listed in
http://gcc.gnu.org/wiki/GCC_Patch_Tracking. In particular,
http://www.reviewboard.org/ (though it seemed too much to me, and I
was looking something similar to our internal review tool).
Diego.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] Fwd: Proposal: Improving patch tracking and review using Rietveld
2011-01-27 7:55 [Qemu-devel] Fwd: Proposal: Improving patch tracking and review using Rietveld Paolo Bonzini
2011-01-27 10:19 ` Stefan Hajnoczi
@ 2011-01-27 17:31 ` Anthony Liguori
2011-01-27 18:32 ` Peter Maydell
2011-01-27 19:40 ` Stefan Hajnoczi
1 sibling, 2 replies; 9+ messages in thread
From: Anthony Liguori @ 2011-01-27 17:31 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On 01/27/2011 01:55 AM, Paolo Bonzini wrote:
> Forwarding this from the GCC mailing list. Since patchwork isn't more
> than a mail archive the way it's implemented in QEMU, this may be a
> more interesting possibility.
Patchwork is a nice tool but I found a few issues with it that really
deterred me from using it:
1) it's all or nothing in terms of whether maintainers use it. if
everyone isn't on top of keeping it clean, you end up with a terrible
backlog
2) it doesn't understand patches series. A 20 patch series gets applied
all at once, yet you have to update status for each patch. That's annoying.
3) it doesn't understand new revisions of the same patch/series. This
is really a deal breaker. Having to go and update the status
particularly when you have patch series that see multiple revs in 24
hours creates an awful lot of work
> Paolo
>
>> At Google we use a code review tool which was open sourced a couple of
>> years ago: Rietveld
>> (http://code.google.com/appengine/articles/rietveld.html).
>>
>> The best way of thinking about it is "bugzilla for patches". The
>> system creates an entry for every patch submitted, provides a web tool
>> for manipulating the patch (comments, different views of the diff,
>> highlighting, etc) and it also has an email gateway.
>>
>> We have discussed patch tracking mechanisms in the past, and none so
>> far has taken hold. The reason why I like Rietveld is that it doesn't
>> really matter whether we all switch to using it at once:
>>
>> 1- Rietveld always send the patch sent to it to gcc-patches@ (provided
>> the submitter added gcc-patches to the CC list).
>> 2- The whole trail of discussion on the patch also get sent to
>> gcc-patches and everyone else is CC'd in it.
>> 3- Reviewers do not need to use the web tool to reply to the patch.
>> One can simply respond to the e-mail, and it will get added to the
>> patch discussion trail.
>>
>> So, for people who do not want to use the tool, Rietveld will not get
>> in the way. They can simply respond to the patch as usual, and as
>> long as they keep the rietveld email address in the CC list, the patch
>> trail will be updated automatically.
>>
>> At Google we will start using Rietveld to send patches. The only
>> difference folks will notice is that Rietveld-generated email has some
>> extra text.
>>
>> I have created a wiki page that explains the basics of using Rietveld
>> (thanks Jeffrey for the instructions):
>> http://gcc.gnu.org/wiki/rietveld
Interesting. This seems to have nice characteristics compared to
patchworks.
Regards,
Anthony Liguori
>> Once again, I'd like to underscore the fact that if a patch submitter
>> chooses to use Rietveld for tracking their patches, this should not
>> affect in any way the traditional mail-based review. All I ask is
>> that reviewers maintain the CC and Subject line intact in order to not
>> confuse the tool.
>>
>> Jeffrey, would you mind looking over the instructions I've written to
>> make sure they're correct?
>>
>> Richard, this is the tool I mentioned in today's chat.
>>
>>
>> Thanks. Diego.
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] Fwd: Proposal: Improving patch tracking and review using Rietveld
2011-01-27 17:31 ` [Qemu-devel] " Anthony Liguori
@ 2011-01-27 18:32 ` Peter Maydell
2011-01-27 19:40 ` Stefan Hajnoczi
1 sibling, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2011-01-27 18:32 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Paolo Bonzini, qemu-devel
On 27 January 2011 17:31, Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:
> Patchwork is a nice tool but I found a few issues with it that really
> deterred me from using it:
> 2) it doesn't understand patches series. A 20 patch series gets applied all
> at once, yet you have to update status for each patch. That's annoying.
I submitted that as a feature request to the patchwork mailing
list earlier this month:
http://lists.ozlabs.org/pipermail/patchwork/2011-January/000348.html
and it didn't get shot down in flames so maybe if we procrastinate
long enough patchwork will improve :-)
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] Fwd: Proposal: Improving patch tracking and review using Rietveld
2011-01-27 17:31 ` [Qemu-devel] " Anthony Liguori
2011-01-27 18:32 ` Peter Maydell
@ 2011-01-27 19:40 ` Stefan Hajnoczi
1 sibling, 0 replies; 9+ messages in thread
From: Stefan Hajnoczi @ 2011-01-27 19:40 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Paolo Bonzini, qemu-devel
On Thu, Jan 27, 2011 at 5:31 PM, Anthony Liguori
<aliguori@linux.vnet.ibm.com> wrote:
> Patchwork is a nice tool but I found a few issues with it that really
> deterred me from using it:
>
> 1) it's all or nothing in terms of whether maintainers use it. if everyone
> isn't on top of keeping it clean, you end up with a terrible backlog
>
> 2) it doesn't understand patches series. A 20 patch series gets applied all
> at once, yet you have to update status for each patch. That's annoying.
You shouldn't have to update patch status manually at all. If there
is a manual step involved then it is unlikely that patchwork's view
will ever stay in sync with the actual repository.
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-01-28 7:18 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-27 7:55 [Qemu-devel] Fwd: Proposal: Improving patch tracking and review using Rietveld Paolo Bonzini
2011-01-27 10:19 ` Stefan Hajnoczi
2011-01-27 10:23 ` Paolo Bonzini
2011-01-27 10:34 ` Stefan Hajnoczi
2011-01-27 16:26 ` [Qemu-devel] " Paolo Bonzini
2011-01-27 16:31 ` Diego Novillo
2011-01-27 17:31 ` [Qemu-devel] " Anthony Liguori
2011-01-27 18:32 ` Peter Maydell
2011-01-27 19:40 ` Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.