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

[Christophe de Dinechin]

> On 21 Jul 2017, at 12:41, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
>> 
>> On Fri, Jul 21, 2017 at 06:18:49AM -0400, Frediano Ziglio wrote:
>>> Beside that I wonder why I had to wait 8 months for these reviews,
>>> not counting the time to decide to rewrite this part of code
>>> (with all the wasted time trying to not do it) and the time we
>>> waited to fix a known bug which is also a regression (image copy
>>> paste are not working) and potentially a security risk (the library
>>> versions used contain security bugs but actually the code is disabled).
>>> Looks like that if a Red Hat client is not pushing for a fix
>>> solutions are not that important.
>> 
>> People can be busy with other things, and patches can fall through the
>> cracks, it's ok to send a 'ping' message once in a while to bring a
>> patch series  back to people's attention.
>> 
>> Christophe
>> 
> 
> I'm really getting tired of this reply..

Hmmm, maybe we can do a few things to reduce that problem.
It would be nice if we had a way to get back to unreviewed patches
without having to browse through the mailing list.

Also, as a newbie, I must say I often spend quite some time finding
the base for a patch, which is a shame, because that’s a problem
that git URLs solve quite nicely. The problem is amplified by the
fact that we have multiple components, and patches sometimes
don’t really specify which component they apply to.

So maybe we can fix the two problems with one stone. What about

1. Pushing branches for review under a consistent name, e.g. something
like ‘review/c3d/recorder’

2. Adding the git URL in the patch, e.g. https://cgit.freedesktop.org/spice/spice-gtk/log/?h=review/c3d/recorder.

3. Once the patch has been committed, simply do a ‘git push freedesktop :review/c3d/recorder’
to delete the branch.


I see several benefits to doing this:

1. We always know exactly which component and branch is being patched

2. When you work on branch, a daily “git fetch” will fetch all the new “review”
branches, so a simple git branch -a will show you what needs to be reviewed.

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)


Do you think something like this would help?


Christophe

> 
> Frediano