On Wed, 05/21 10:23, Kevin Wolf wrote: > Am 21.05.2014 um 09:46 hat Markus Armbruster geschrieben: > > Fam Zheng <[email protected]> writes: > > > > > On Wed, 05/21 07:54, Markus Armbruster wrote: > > >> Fam Zheng <[email protected]> writes: > > >> > > >> > On Tue, 05/20 13:13, Eric Blake wrote: > > >> >> On 05/20/2014 03:07 AM, Fam Zheng wrote: > > >> >> > Please first take a look at patch 7 to see what is supported by > > >> >> > this series. > > >> >> > > > >> >> > Patch 1 ~ 3 allows some useful basic types in schema. > > >> >> > > > >> >> > Patch 4 ~ 6 implements the new syntax. > > >> >> > > > >> >> > Note: The introduced '@arg' sigil, just like the preexisting > > >> >> > '*arg', is > > >> >> > reducing the cleanness of the syntax. We should get rid of both of > > >> >> > them in long > > >> >> > term. Here, this series compromises on this and introduces '@arg' > > >> >> > because: > > >> >> > > > >> >> > - We have to distinguish the argument property dictionary from > > >> >> > nested struct: > > >> >> > > > >> >> > I.e.: > > >> >> > > > >> >> > 'data': { > > >> >> > 'arg1': { 'member1': 'int', 'member2': 'str' } > > >> >> > '@arg2': { 'type': 'int', 'default': 100 } > > >> >> > } > > >> >> > > > >> >> > Until we completely drop and forbid the 'arg1' nested struct > > >> >> > use case. > > >> >> > > > >> >> > - Forbidding 'arg1' it's doable, but doing it now means we pull > > >> >> > in many > > >> >> > distractive patches to this series. > > >> >> > > >> >> Question - since we WANT to get rid of nested struct, why not reverse > > >> >> the sense? Mark all existing nested structs (weren't there just three > > >> >> that we found?) with the '@' sigil, and let the new syntax be > > >> >> sigil-free. Then when we clean up the nesting, we are also getting > > >> >> rid > > >> >> of the bad syntax, plus the sigil gives us something to search for in > > >> >> knowing how much to clean up. But if you stick the sigil on the new > > >> >> code, instead of the obsolete code, then as more and more places in > > >> >> the > > >> >> schema use defaults, it gets harder and harder to remove the use of > > >> >> the > > >> >> sigil even if the nested structs are eventually removed. > > >> >> > > >> > > > >> > It makes not much difference I can see. The hard part is actaully > > >> > dropping > > >> > nested, converting from sigil <-> non-sigil is easy. Of course, > > >> > nothing is > > >> > seriously hard, there are only three nested structs plus some more > > >> > qapi-schema > > >> > test code. > > >> > > >> 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? 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. Fam
