Re: [Qemu-devel] [PATCH 1/7] tests: qgraph API for the qtest driver framework

[Stefan Hajnoczi]

On Wed, Jul 18, 2018 at 11:13:18PM +0200, Emanuele wrote:
> 
> 
> On 07/18/2018 09:28 PM, Paolo Bonzini wrote:
> > On 18/07/2018 16:23, Stefan Hajnoczi wrote:
> > > > > > +struct QOSGraphObject {
> > > > > > +    /* for produces, returns void * */
> > > > > > +    QOSGetDriver get_driver;
> > > > > Unused?
> > > > > 
> > > > > > +    /* for contains, returns a QOSGraphObject * */
> > > > > > +    QOSGetDevice get_device;
> > > > > Unused?
> > > > What is unused?
> > > Neither of these fields are used in this patch.  Please introduce them
> > > in the first patch that actually uses them.  This way code review can
> > > proceed linearly and it also prevents deadcode when just part of a patch
> > > series is merged or backported.
> > So do you suggest to squash patch 6 into this one, so that a user of
> > QOSGraphObject exists already here?
> I think he's right, it makes no sense to have qos-graph as patch 6, it could
> go as patch 2, even though by that time no object/node exist yet.
> 
> Moreover, right now no file in patch 3-4-5 is actually compiled until patch
> 6, which is easily prone to errors in case of future edits (I too have this
> problem right now, when I need to modify sdhci.c and I'm forced to return to
> the branch head to compile), so adding it before would make things more
> clear.

Sounds good.

Stefan

signature.asc
Description: PGP signature