[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v14 01/19] qapi: Consolidate object visitors

From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v14 01/19] qapi: Consolidate object visitors
Date: Fri, 15 Apr 2016 17:05:24 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Eric Blake <address@hidden> writes:

> On 04/13/2016 06:48 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>> Rather than having two separate visitor callbacks with items
>>> already broken out, pass the actual QAPISchemaObjectType object
>>> to the visitor.  This lets the visitor access things like
>>> type.is_implicit() without needing another parameter, resolving
>>> a TODO from previous patches.
>>> For convenience and consistency, the 'name' and 'info' parameters
>>> are still provided, even though they are now redundant with
>>> 'typ.name' and 'typ.info'.
>>> Signed-off-by: Eric Blake <address@hidden>
>> I've made you push this one back in the queue a couple of times, because
>> there are pros and cons, and the work at hand didn't actually require
>> the patch.  At some point we need to decide.  Perhaps that point is now.
>> The patch replaces two somewhat unclean "is implicit" tests by clean
>> .is_implicit() calls.  Any other use of the change in this series?
> I'm not seeing any other direct simplification in this series.  As a
> quick test, I just rebased it to the end of this series with no merge
> conflicts, and everything else still compiled and passed without it.
> I'm less sure whether any of my later pending series depend on it.
>> Recap of pros and cons:
>> * The existing interface
>>       def visit_object_type(self, name, info, base, members, variants):
>>       def visit_object_type_flat(self, name, info, members, variants):
>>   is explicit and narrow, but when you need more information, you have
>>   to add parameters or functions.
>> * The new interface
>>      def visit_object_type(self, name, info, typ):
>>   avoids that, but now its users can access everything.
>> This patch touches only visiting of objects, because only for objects we
>> have a TODO.  Should we change the other visit_ methods as well, for
>> consistency?
> I have a pending patch in subset F (last posted at v6) that adds a 'box'
> parameter to visit_event and visit_command:
> https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04397.html
> If we change all the other visit_ methods for consistency, then those
> methods would directly access command.box and event.box instead of
> needing to add a separate parameter.

Let's move this patch into subset F (should be the next series, as this
one is E), and figure it out there.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]