From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60039) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ar5JN-0001FE-N9 for qemu-devel@nongnu.org; Fri, 15 Apr 2016 11:05:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ar5JJ-0002zE-Cu for qemu-devel@nongnu.org; Fri, 15 Apr 2016 11:05:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33810) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ar5JJ-0002zA-64 for qemu-devel@nongnu.org; Fri, 15 Apr 2016 11:05:29 -0400 From: Markus Armbruster References: <1460131992-32278-1-git-send-email-eblake@redhat.com> <1460131992-32278-2-git-send-email-eblake@redhat.com> <878u0hn08l.fsf@dusky.pond.sub.org> <570E7047.3000600@redhat.com> Date: Fri, 15 Apr 2016 17:05:24 +0200 In-Reply-To: <570E7047.3000600@redhat.com> (Eric Blake's message of "Wed, 13 Apr 2016 10:13:59 -0600") Message-ID: <87d1pqoqtn.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v14 01/19] qapi: Consolidate object visitors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, Michael Roth Eric Blake writes: > On 04/13/2016 06:48 AM, Markus Armbruster wrote: >> Eric Blake 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 >> >> 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.