On 02/21/2014 01:21 AM, Markus Armbruster wrote: >> I'd still rather make it explicit that we KNOW that this branch of the >> union has no additional options: >> >> { 'union': 'FooOptions', >> 'base': 'CommonFooOptions', >> 'discriminator': 'type', >> 'data': { 'plain': {}, >> 'bells': 'BellsOptions', >> 'whistles': 'WhistlesOptions' } } >> >> to show that we explicitly thought about all the cases. We don't >> currently have any such unions with an empty branch, but it would be >> worth documenting in the qapi text and explicitly testing that it works >> if we intend to support this. > > Fair point. However, it requires 'plain': {} to work, and it doesn't in > my testing.
> We should extend the generator to permit {} before we insist on unions > covering all discriminator values explicitly. Because if we don't, > people will be compelled to add dummy fields. So far, we have no one that should be omitting a branch. But yes, I agree that we should update the generator to allow an empty branch. In the context of _this_ patch, requiring that all branches are covered doesn't hurt until a future patch actually has to add a dummy field or fix the generator. I don't know if it's something Wenchao would like to add in the next spin, or if we should just live this this patch being a little over-ambitious on what it enforces in the absence of more generic support for an explicit empty branch. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature