Re: [Qemu-devel] [PATCH v4 06/51] qapi: pass 'if' condition into QAPISchemaEntity objects
Marc-André Lureau writes: > Hi > > On Tue, Feb 6, 2018 at 11:12 AM, Markus Armbruster wrote: >> Marc-André Lureau writes: >> >>> Built-in objects remain unconditional. Explicitly defined objects >>> use the condition specified in the schema. Implicitly defined >>> objects inherit their condition from their users. For most of them, >>> there is exactly one user, so the condition to use is obvious. The >>> exception is the wrapped type's generated for simple union variants, >>> which can be shared by any number of simple unions. The tight >>> condition would be the disjunction of the conditions of these simple >>> unions. For now, use wrapped type's condition instead. Much >>> simpler and good enough for now. >>> >>> Signed-off-by: Marc-André Lureau >>> Reviewed-by: Markus Armbruster >>> --- >>> scripts/qapi.py | 98 >>> ++--- >>> 1 file changed, 66 insertions(+), 32 deletions(-) >>> >>> diff --git a/scripts/qapi.py b/scripts/qapi.py >>> index 27df0fcf48..8f54dead8d 100644 >>> --- a/scripts/qapi.py >>> +++ b/scripts/qapi.py >>> @@ -991,8 +991,17 @@ def check_exprs(exprs): >>> # Schema compiler frontend >>> # >>> >>> +def listify_cond(ifcond): >>> +if not ifcond: >>> +return [] >>> +elif not isinstance(ifcond, list): >>> +return [ifcond] >>> +else: >>> +return ifcond >> >> pylint complains: >> >> R:995, 4: Unnecessary "else" after "return" (no-else-return) >> >> Matter of taste. Mine happens to agree with pylint's. Suggest: >> >>def listify_cond(ifcond): >>if not ifcond: >>return [] >>if not isinstance(ifcond, list): >>return [ifcond] >>return ifcond >> > > There are so many errors with pylint, that I don't bother running it. pylint reports lots of stuff that is actually just fine. > How did you notice? you run diff of error output between commits? Yes. > pycodestyle --diff is more convenient, and silent on this code. Formerly known as pep8. Confusingly, Fedora 26 packages both separately. Thanks for the pointer. > Feel free to touch if you pick the patch. Okay. >>> + >>> + >>> class QAPISchemaEntity(object): >>> -def __init__(self, name, info, doc): >>> +def __init__(self, name, info, doc, ifcond=None): >>> assert isinstance(name, str) >>> self.name = name >>> # For explicitly defined entities, info points to the (explicit) >> [...] >>
Re: [Qemu-devel] [PATCH v4 06/51] qapi: pass 'if' condition into QAPISchemaEntity objects
Hi On Tue, Feb 6, 2018 at 11:12 AM, Markus Armbruster wrote: > Marc-André Lureau writes: > >> Built-in objects remain unconditional. Explicitly defined objects >> use the condition specified in the schema. Implicitly defined >> objects inherit their condition from their users. For most of them, >> there is exactly one user, so the condition to use is obvious. The >> exception is the wrapped type's generated for simple union variants, >> which can be shared by any number of simple unions. The tight >> condition would be the disjunction of the conditions of these simple >> unions. For now, use wrapped type's condition instead. Much >> simpler and good enough for now. >> >> Signed-off-by: Marc-André Lureau >> Reviewed-by: Markus Armbruster >> --- >> scripts/qapi.py | 98 >> ++--- >> 1 file changed, 66 insertions(+), 32 deletions(-) >> >> diff --git a/scripts/qapi.py b/scripts/qapi.py >> index 27df0fcf48..8f54dead8d 100644 >> --- a/scripts/qapi.py >> +++ b/scripts/qapi.py >> @@ -991,8 +991,17 @@ def check_exprs(exprs): >> # Schema compiler frontend >> # >> >> +def listify_cond(ifcond): >> +if not ifcond: >> +return [] >> +elif not isinstance(ifcond, list): >> +return [ifcond] >> +else: >> +return ifcond > > pylint complains: > > R:995, 4: Unnecessary "else" after "return" (no-else-return) > > Matter of taste. Mine happens to agree with pylint's. Suggest: > >def listify_cond(ifcond): >if not ifcond: >return [] >if not isinstance(ifcond, list): >return [ifcond] >return ifcond > There are so many errors with pylint, that I don't bother running it. How did you notice? you run diff of error output between commits? pycodestyle --diff is more convenient, and silent on this code. Feel free to touch if you pick the patch. >> + >> + >> class QAPISchemaEntity(object): >> -def __init__(self, name, info, doc): >> +def __init__(self, name, info, doc, ifcond=None): >> assert isinstance(name, str) >> self.name = name >> # For explicitly defined entities, info points to the (explicit) > [...] > -- Marc-André Lureau
Re: [Qemu-devel] [PATCH v4 06/51] qapi: pass 'if' condition into QAPISchemaEntity objects
Marc-André Lureau writes: > Built-in objects remain unconditional. Explicitly defined objects > use the condition specified in the schema. Implicitly defined > objects inherit their condition from their users. For most of them, > there is exactly one user, so the condition to use is obvious. The > exception is the wrapped type's generated for simple union variants, > which can be shared by any number of simple unions. The tight > condition would be the disjunction of the conditions of these simple > unions. For now, use wrapped type's condition instead. Much > simpler and good enough for now. > > Signed-off-by: Marc-André Lureau > Reviewed-by: Markus Armbruster > --- > scripts/qapi.py | 98 > ++--- > 1 file changed, 66 insertions(+), 32 deletions(-) > > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 27df0fcf48..8f54dead8d 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -991,8 +991,17 @@ def check_exprs(exprs): > # Schema compiler frontend > # > > +def listify_cond(ifcond): > +if not ifcond: > +return [] > +elif not isinstance(ifcond, list): > +return [ifcond] > +else: > +return ifcond pylint complains: R:995, 4: Unnecessary "else" after "return" (no-else-return) Matter of taste. Mine happens to agree with pylint's. Suggest: def listify_cond(ifcond): if not ifcond: return [] if not isinstance(ifcond, list): return [ifcond] return ifcond > + > + > class QAPISchemaEntity(object): > -def __init__(self, name, info, doc): > +def __init__(self, name, info, doc, ifcond=None): > assert isinstance(name, str) > self.name = name > # For explicitly defined entities, info points to the (explicit) [...]
Re: [Qemu-devel] [PATCH v4 06/51] qapi: pass 'if' condition into QAPISchemaEntity objects
Marc-André Lureau writes: > Built-in objects remain unconditional. Explicitly defined objects > use the condition specified in the schema. Implicitly defined > objects inherit their condition from their users. For most of them, > there is exactly one user, so the condition to use is obvious. The > exception is the wrapped type's generated for simple union variants, Editing accident: you changed "is the wrapper types" to "is the wrapped type's" here instead of > which can be shared by any number of simple unions. The tight > condition would be the disjunction of the conditions of these simple > unions. For now, use wrapped type's condition instead. Much changing "use wrapped type's" to "use the wrapped type's" here. Can touch up on commit. > simpler and good enough for now. > > Signed-off-by: Marc-André Lureau > Reviewed-by: Markus Armbruster R-by stands.
