Re: [Qemu-devel] [RFC PATCH v2 0/7] qapi: Specify default value for optional argument in schema json

[Eric Blake]

On 05/21/2014 02:42 AM, Fam Zheng wrote:

>>>>> Adding three ugly sigils and making everybody include one when they add
>>>>> a nested struct feels much better to me than ugly sigils all over the
>>>>> place.
>>>>
>>>> Well, I could use some background here. Why did we introduce nested 
>>>> structure
>>>> in the first place?
>>>
>>> Because we could?
>>>
>>> Felt like a good idea at the time?
>>>
>>> I quick glance at commit 0f923be and fb3182c suggests they have been
>>> supported since the beginning.  There is no design rationale.
>>
>> Let me extend Fam's question: Why don't we simply remove them right
>> now? If it's really only three instances, converting them to full
>> types should be a matter of five minutes.
>>
> 
> Actually, my question is: do we want it independently, or do we want to 
> include
> the removal of nested as the first part of this series?

Doing it as an independent series first might be the way forward -
independent so that it doesn't stall on reviews of the new syntax for
default values, and up front because it seems like a simple enough
conversion that then makes the entire generator simpler that it will be
easy to approve and get in tree.

> 
> I would prefer the former because I feel uncomfortable with making more 
> changes
> in this series, since there are already many things to do: adding qapi types,
> adding argument property dict, adding all test cases for all of them, updating
> documentation, and apply the new syntax in qapi-schema.json. A non-RFC 
> revision
> could be long and hard to review.

At the end of the day, we want both things; and it makes more sense to
remove the conflicting syntax up front than it does to add an alternate
syntax only to later remove it.  Doing it as two shorter series one
after the other rather than cramming it into one long series is
psychologically easier to review.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature