[Spice-devel] Proposal: review branches (was Re: [vdagent-win PATCH v6 2/5] Initial rewrite of image conversion code)

[Christophe Fergeau]

On Tue, Jul 25, 2017 at 07:09:22AM -0400, Frediano Ziglio wrote:
> > 
> > > >>> I see several benefits to doing this:
> > > >>> 
> > > >>> 1. We always know exactly which component and branch is being patched
> > > >>> 
> > > > 
> > > > As long as contributor keep pinging or resending his series, this is
> > > > already the case.
> > > 
> > > As Frediano said at the beginning of the series, “I’m tired of hearing this
> > > reply”.
> > 
> > And this is not an actionable answer... My perception is that there
> > rarely are 'ping' on old series. Does this mean we are doing a good job
> > at reviews? (I doubt it or we would not have this conversation) Does
> > this mean patch senders do not want to do that? Why? Does this mean it's
> > done a lot, but to no avail? All I'm reading is "I'm not happy with how
> > things work", with nothing specific.
> > 
> 
> Patch series are getting old (even years) repeatedly pinged (5/6 times)
> but they continue to not getting any feedback/ack/comment.
> If you can't remember any... this just confirms the problem.

So I went through my mails (searched for mails containing 'ping'), in
the last months I found 24 series which needed a ping, among these, 3
needed several pings, and it stopped at 2 pings. Maybe I missed some.

> 
> > > 
> > > > 
> > > >>> 3. If you want to test, a git checkout is enough to test (assuming you
> > > >>> did
> > > >>> the git fetch above). Simpler IMO than git am (even if I assume most of
> > > >>> us
> > > >>> have scripts to process incoming mail)
> > > > 
> > > > qemu uses patchew, I think it would be worth to consider it for
> > > > spice as well. It applies the patches on a mailing list, run some
> > > > tests, gives you a stable URL, tracks the review (and the various
> > > > iteration recently iirc)…
> > > 
> > > Good tool, but different problem.
> > 
> > I personally don't know which exact problem we are tackling ;) I see
> > some people having issues keeping track of pending series, others saying
> > that CI would be nice, ... Are we trying to solve one single very specific
> > problem? If yes, what is it? Patch reviews not being done in a timely
> > manner? Patch series being forgotten? Patch series status hard to know
> > by email? Something else? (note that you said "problem", not "problems"
> > :)
> > 
> > Christophe
> > 
> 
> If the review is done months after been sent to the ML the author
> has to review again the patches, not counting the time he has to
> spend to update the series and test it again (as the master in the
> meantime has moved).
> 
> Just for instance the png patches were sent on November and
> are being seriously reviewed this month (so 8 months after).
> I agree in this case where not pinged that much there were
> also different discussion on them about.
> There are a lot of stuff that get lost if the review is done
> so late. Think about the searches done on the web, the experiments
> before coming to that version, the commands used which could be
> helpful again, talks with other people.

Ok, I take from that email that the main issue is untimely reviews, with
which I agree. However, what we are currently discussing is how to
better keep track of pending patch series. If the reason for the
untimely reviews is that people are not aware that series A and B need
reviewing, then I agree this is going to help. If the reason for the
delayed reviews is because we don't have enough reviewers, no tools are
going to help us here.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170725/7c4a4f08/attachment.sig>