Message ID | 20170720162815.19802-8-ldoktor@redhat.com |
---|---|
State | New |
Headers | show |
On Thu, Jul 20, 2017 at 06:28:11PM +0200, Lukáš Doktor wrote: > There is no need to define QEMUMonitorProtocol as old-style class. > > Signed-off-by: Lukáš Doktor <ldoktor@redhat.com> > --- > scripts/qmp/qmp.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py > index bb4d614..68f3420 100644 > --- a/scripts/qmp/qmp.py > +++ b/scripts/qmp/qmp.py > @@ -30,7 +30,7 @@ class QMPTimeoutError(QMPError): > pass > > > -class QEMUMonitorProtocol: > +class QEMUMonitorProtocol(object): I don't fully understand the consequences of changing to new-style classes when using old-style SuperClass.__init__() calls in the __init__(). Should we change QMPShell.__init__() to use super()? > > #: Socket's error class > error = socket.error > -- > 2.9.4 >
Dne 20.7.2017 v 20:35 Eduardo Habkost napsal(a): > On Thu, Jul 20, 2017 at 06:28:11PM +0200, Lukáš Doktor wrote: >> There is no need to define QEMUMonitorProtocol as old-style class. >> >> Signed-off-by: Lukáš Doktor <ldoktor@redhat.com> >> --- >> scripts/qmp/qmp.py | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py >> index bb4d614..68f3420 100644 >> --- a/scripts/qmp/qmp.py >> +++ b/scripts/qmp/qmp.py >> @@ -30,7 +30,7 @@ class QMPTimeoutError(QMPError): >> pass >> >> >> -class QEMUMonitorProtocol: >> +class QEMUMonitorProtocol(object): > > I don't fully understand the consequences of changing to > new-style classes when using old-style SuperClass.__init__() > calls in the __init__(). Should we change QMPShell.__init__() to > use super()? > The consequence is it becomes a proper object with full object model and less workarounds. It also consumes a bit more memory but are the only available mode in py3. As for the old-style superclass, it works, but the correct approach is to replace it with `super` call. I'll address that in the v2 (I only checked for `.py` files but there are many python sources in qemu tree without the proper extension. I still need to get used to this.). Lukáš > >> >> #: Socket's error class >> error = socket.error >> -- >> 2.9.4 >> >
On Fri, Jul 21, 2017 at 08:50:22AM +0200, Lukáš Doktor wrote: > Dne 20.7.2017 v 20:35 Eduardo Habkost napsal(a): > > On Thu, Jul 20, 2017 at 06:28:11PM +0200, Lukáš Doktor wrote: > >> There is no need to define QEMUMonitorProtocol as old-style class. > >> > >> Signed-off-by: Lukáš Doktor <ldoktor@redhat.com> > >> --- > >> scripts/qmp/qmp.py | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py > >> index bb4d614..68f3420 100644 > >> --- a/scripts/qmp/qmp.py > >> +++ b/scripts/qmp/qmp.py > >> @@ -30,7 +30,7 @@ class QMPTimeoutError(QMPError): > >> pass > >> > >> > >> -class QEMUMonitorProtocol: > >> +class QEMUMonitorProtocol(object): > > > > I don't fully understand the consequences of changing to > > new-style classes when using old-style SuperClass.__init__() > > calls in the __init__(). Should we change QMPShell.__init__() to > > use super()? > > > The consequence is it becomes a proper object with full object > model and less workarounds. It also consumes a bit more memory > but are the only available mode in py3. > > As for the old-style superclass, it works, but the correct > approach is to replace it with `super` call. I'll address that > in the v2 (I only checked for `.py` files but there are many > python sources in qemu tree without the proper extension. I > still need to get used to this.). Thanks for the explanation. Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py index bb4d614..68f3420 100644 --- a/scripts/qmp/qmp.py +++ b/scripts/qmp/qmp.py @@ -30,7 +30,7 @@ class QMPTimeoutError(QMPError): pass -class QEMUMonitorProtocol: +class QEMUMonitorProtocol(object): #: Socket's error class error = socket.error
There is no need to define QEMUMonitorProtocol as old-style class. Signed-off-by: Lukáš Doktor <ldoktor@redhat.com> --- scripts/qmp/qmp.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)