[Spice-devel] [PATCH spice-common] Integrate recorder library

[Frediano Ziglio]

> Hey,
> 
> On Thu, Nov 29, 2018 at 08:50:47AM -0500, Frediano Ziglio wrote:
> > > 
> > > On Wed, Nov 28, 2018 at 11:23:17AM +0100, Christophe de Dinechin wrote:
> > > > > On 27 Nov 2018, at 15:38, Christophe Fergeau <cfergeau at redhat.com>
> > > > > wrote:
> > > > > I'm not really asking how to use it, but it's very odd to have a
> > > > > patch
> > > > > adding a new dep without seeing any accompanying patches which need
> > > > > that
> > > > > dependency.
> > > > 
> > > > You may want to look there:
> > > > https://github.com/c3d/spice/tree/smart-streaming.
> > > > I’m surprised you are not aware of this proposal.
> > > 
> > > I'm not saying I'm unable to find any examples of how it's used. By
> > > looking at the patch sent to the mailing list, I have no idea if I
> > > should assume the instrumentation is going to be merged as is, if only a
> > > subset of it is intended to be submitted, if it's going to be modified,
> > > ... So yes I'm aware of your work, but I don't know for sure what's
> > > the plan for upstream.
> > > 
> > > Christophe
> > > 
> > 
> > The plan is the same for downstream, downstream is mostly some former
> > release with some patches on top taken from newer upstream (like CVEs
> > and other important fixes), the only exception, as far as I know, are
> > some customer related not still integrated to upstream that will be
> > merged upstream when ready.
> > 
> > Why should be different for this patch?
> > (Probably I didn't understand your concern)
> 
> I don't really understand what you mean here, so I guess we are indeed
> not understanding each other. What I'm trying to say as that it's
> quite unusual to have a patch which is "Add a new dependency" without
> any accompanying patch making use of that new dependency.
> 

I think I was confused by the "upstream" and though that your concern
was related to some upstream/downstream difference.

> For example, when Marc-André added the json-glib dependency (
> https://lists.freedesktop.org/archives/spice-devel/2018-August/045202.html
> ), we did not get first a patch adding the new dep and not doing
> anythingelse, and the rest of the series after this initial patch was
> merged. The addition of the new dep was sent at the same time as the
> patches which need json-glib.
> 

Usually following patches are also an explanation of the stuff you are
adding. In this case the usage, as you also said, is pretty clear.
A difference of this patch is that is not a simple library addition,
there's an option, a dummy replacement, a test and configure function.

What you probably want to say is "I'd prefer the merge to be done
with some code really making use of this library". If that the case
however should not be an excuse to avoid to review it and discuss.

> Christophe
> 

Frediano