From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41270) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YbTZD-0003en-E8 for qemu-devel@nongnu.org; Fri, 27 Mar 2015 08:40:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YbTZ8-00079S-Bs for qemu-devel@nongnu.org; Fri, 27 Mar 2015 08:40:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37516) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YbTZ8-00078r-3f for qemu-devel@nongnu.org; Fri, 27 Mar 2015 08:40:46 -0400 From: Markus Armbruster References: <1427227433-5030-1-git-send-email-eblake@redhat.com> <1427227433-5030-19-git-send-email-eblake@redhat.com> <87wq23wte4.fsf@blackfin.pond.sub.org> <55145872.6070005@redhat.com> Date: Fri, 27 Mar 2015 13:40:42 +0100 In-Reply-To: <55145872.6070005@redhat.com> (Eric Blake's message of "Thu, 26 Mar 2015 13:05:22 -0600") Message-ID: <871tkalik5.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v5 18/28] qapi: Unify type bypass and add tests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: kwolf@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, wenchaoqemu@gmail.com, lcapitulino@redhat.com Eric Blake writes: > On 03/26/2015 11:38 AM, Markus Armbruster wrote: >> Eric Blake writes: >> >>> For a few QMP commands, we are forced to pass an arbitrary type >>> without tracking it properly in QAPI. Among the existing clients, >>> this unnamed type was spelled 'dict', 'visitor', and '**'; this >>> patch standardizes on '**'. >>> >>> Meanwhile, for both 'gen' and 'success-response' keys, we have been >>> ignoring the value, although the schema consistently used "'no'". >>> But now that we can support a literal "false" in the schema, we >>> might as well use that rather than ignoring the value or >>> special-casing a random string. >>> >>> There is no difference to the generated code. As these features >>> were previously undocumented before this series, add some tests >>> and documentation on what we'd like to guarantee, although it will >>> take later patches to clean up test results and actually enforce >>> the use of a bool parameter. >> >> You don't actually add documentation in this patch. > > Hmm, more evidence that I waffled about per-commit doc fixes, vs. > lumping it all in patch 1, and I obviously failed to scrub the commit > messages after changing my mind. Fortunately, commit messages don't need testing, so this is easy enough to fix in a respin :) >> Aside: 'gen': false is required when '**' is used anywhere in the >> command. If it was permitted only then, it would be redundant. I think >> we happily accept 'gen': false without '**' so far, although we don't >> use it. That's okay. > > Also, even though the code accepts 'gen':false, it rejects 'gen':true > ('gen' is only a one-way switch away from the default). Also something > I didn't think worth worrying about. Agree. >>> Signed-off-by: Eric Blake >> >> Reviewed-by: Markus Armbruster >> >>