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

Frediano Ziglio fziglio at redhat.com
Fri Jul 28 08:23:18 UTC 2017


> 
> > On 27 Jul 2017, at 17:08, Frediano Ziglio <fziglio at redhat.com> wrote:
> > 
> > Try to sum up the initial problem was patches/series tracking
> > 
> > So far there are 3 proposal
> > 1) PR/MR (GitLab/GitHub style)
> > 2) patchew
> > 3a) shared git repository
> > 3b) links to external git repositories
> > 
> > 1) PR surely can trace the status of series and is ready to
> >   use with small initial setup. Not clear if we would like
> >   to do reviews with it;
> 
> Not many answers to my survey yet, but so far a consensus that
> patches should be sent to the mailing list, which I interpret as
> an indication that people are comfortable with mail-based reviews.
> 

No, there is IMO a big distinction. If you open a PR and
you get a mail with the patch this does not mean you can
review using the ML. Reviewing using the ML require you
the ability to reply to these emails.
Not saying this is a possible flow to go just that saying
that "patches sent to ML" does not mean "ML review".

> 
> > 2) similar to patchwork with additional feature but missing
> >   the state tracking part. Maybe would be not hard to add;
> 
> To me, addresses a different issue, so I would propose both 1 and 2.
> Specifically, 1 addresses the server side (managing CI items, list of
> branches
> under review, build status, etc), whereas 2 addresses the mail side
> (turning patches into “CI items”).
> 

CI items? CI is usually bound to compilation/testing, not strictly
patches/series.

> 
> > 3a) Many disagree as not really git ideal and about
> >    external contributions;
> > 3b) Does this improve knowing the state of series? Maybe
> >    for internal developers only.
> 
> I think that 1 implies 3, doesn’t it?
> 

1 implies "external git repositories", not "links to external git repositories".

> > 
> > 3a/3b seems quite manual job to do and not much solving
> > the state tracking (although solve other issues), maybe
> > some ideas could improve the current procedure.
> > 
> > Maybe would be worth speaking with patchew author if
> > is easy doable and agree with the change.
> > 
> > IMHO the "closest" (more suitable and easy to implement)
> > is 1.
> 
> Agreed. But I think 2 would be a valuable addition, notably as
> an efficient way to deal with smaller patches.
> 

Explain. Why small patches should not have a PR?

> 
> Thanks,
> Christophe
> > 

--
Frediano


More information about the Spice-devel mailing list