diff mbox

[10/11] qtest.py: Few pylint/style fixes

Message ID 20170720162815.19802-11-ldoktor@redhat.com
State New
Headers show

Commit Message

Lukáš Doktor July 20, 2017, 4:28 p.m. UTC
No actual code changes, just few pylint/style fixes.

Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
---
 scripts/qtest.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Eduardo Habkost July 20, 2017, 6:42 p.m. UTC | #1
On Thu, Jul 20, 2017 at 06:28:14PM +0200, Lukáš Doktor wrote:
[...]
> @@ -83,8 +80,11 @@ class QEMUQtestMachine(qemu.QEMUMachine):
>                   socket_scm_helper=None):
>          if name is None:
>              name = "qemu-%d" % os.getpid()
> -        super(QEMUQtestMachine, self).__init__(binary, args, name=name, test_dir=test_dir,
> -                                               socket_scm_helper=socket_scm_helper)
> +        scm_helper = socket_scm_helper

Why is this necessary?

> +        super(QEMUQtestMachine, self).__init__(binary, args, name=name,
> +                                               test_dir=test_dir,
> +                                               socket_scm_helper=scm_helper)
> +        self._qtest = None
>          self._qtest_path = os.path.join(test_dir, name + "-qtest.sock")
>  
>      def _base_args(self):
> -- 
> 2.9.4
>
Lukáš Doktor July 21, 2017, 6:57 a.m. UTC | #2
Dne 20.7.2017 v 20:42 Eduardo Habkost napsal(a):
> On Thu, Jul 20, 2017 at 06:28:14PM +0200, Lukáš Doktor wrote:
> [...]
>> @@ -83,8 +80,11 @@ class QEMUQtestMachine(qemu.QEMUMachine):
>>                   socket_scm_helper=None):
>>          if name is None:
>>              name = "qemu-%d" % os.getpid()
>> -        super(QEMUQtestMachine, self).__init__(binary, args, name=name, test_dir=test_dir,
>> -                                               socket_scm_helper=socket_scm_helper)
>> +        scm_helper = socket_scm_helper
> 
> Why is this necessary?
> 
to avoid > 80 chars line. It should be optimized-out by the python compiler so it should not slow down the execution. Alternative solution is to use:

    super(QEMUQtestMachine,
          self.__init__(...)

which looks IMO uglier, but I can use that in v2, should that be your preferred style.

Lukáš

>> +        super(QEMUQtestMachine, self).__init__(binary, args, name=name,
>> +                                               test_dir=test_dir,
>> +                                               socket_scm_helper=scm_helper)
>> +        self._qtest = None
>>          self._qtest_path = os.path.join(test_dir, name + "-qtest.sock")
>>  
>>      def _base_args(self):
>> -- 
>> 2.9.4
>>
>
Eduardo Habkost July 21, 2017, 6:56 p.m. UTC | #3
On Fri, Jul 21, 2017 at 08:57:34AM +0200, Lukáš Doktor wrote:
> Dne 20.7.2017 v 20:42 Eduardo Habkost napsal(a):
> > On Thu, Jul 20, 2017 at 06:28:14PM +0200, Lukáš Doktor wrote:
> > [...]
> >> @@ -83,8 +80,11 @@ class QEMUQtestMachine(qemu.QEMUMachine):
> >>                   socket_scm_helper=None):
> >>          if name is None:
> >>              name = "qemu-%d" % os.getpid()
> >> -        super(QEMUQtestMachine, self).__init__(binary, args, name=name, test_dir=test_dir,
> >> -                                               socket_scm_helper=socket_scm_helper)
> >> +        scm_helper = socket_scm_helper
> > 
> > Why is this necessary?
> > 
> to avoid > 80 chars line. It should be optimized-out by the
> python compiler so it should not slow down the execution.
> Alternative solution is to use:
> 
>     super(QEMUQtestMachine,
>           self.__init__(...)
> 
> which looks IMO uglier, but I can use that in v2, should that be your preferred style.

I think that would be better.  The purpose of the extra variable
isn't clear when reading the code, making it more confusing.
Lukáš Doktor July 24, 2017, 12:42 p.m. UTC | #4
Dne 21.7.2017 v 20:56 Eduardo Habkost napsal(a):
> On Fri, Jul 21, 2017 at 08:57:34AM +0200, Lukáš Doktor wrote:
>> Dne 20.7.2017 v 20:42 Eduardo Habkost napsal(a):
>>> On Thu, Jul 20, 2017 at 06:28:14PM +0200, Lukáš Doktor wrote:
>>> [...]
>>>> @@ -83,8 +80,11 @@ class QEMUQtestMachine(qemu.QEMUMachine):
>>>>                   socket_scm_helper=None):
>>>>          if name is None:
>>>>              name = "qemu-%d" % os.getpid()
>>>> -        super(QEMUQtestMachine, self).__init__(binary, args, name=name, test_dir=test_dir,
>>>> -                                               socket_scm_helper=socket_scm_helper)
>>>> +        scm_helper = socket_scm_helper
>>>
>>> Why is this necessary?
>>>
>> to avoid > 80 chars line. It should be optimized-out by the
>> python compiler so it should not slow down the execution.
>> Alternative solution is to use:
>>
>>     super(QEMUQtestMachine,
>>           self.__init__(...)
>>
>> which looks IMO uglier, but I can use that in v2, should that be your preferred style.
> 
> I think that would be better.  The purpose of the extra variable
> isn't clear when reading the code, making it more confusing.
> 

OK, will fix in v2.
Lukáš
diff mbox

Patch

diff --git a/scripts/qtest.py b/scripts/qtest.py
index d5aecb5..e4b5788 100644
--- a/scripts/qtest.py
+++ b/scripts/qtest.py
@@ -11,14 +11,11 @@ 
 # Based on qmp.py.
 #
 
-import errno
 import socket
-import string
 import os
-import subprocess
-import qmp.qmp
 import qemu
 
+
 class QEMUQtestProtocol(object):
     def __init__(self, address, server=False):
         """
@@ -83,8 +80,11 @@  class QEMUQtestMachine(qemu.QEMUMachine):
                  socket_scm_helper=None):
         if name is None:
             name = "qemu-%d" % os.getpid()
-        super(QEMUQtestMachine, self).__init__(binary, args, name=name, test_dir=test_dir,
-                                               socket_scm_helper=socket_scm_helper)
+        scm_helper = socket_scm_helper
+        super(QEMUQtestMachine, self).__init__(binary, args, name=name,
+                                               test_dir=test_dir,
+                                               socket_scm_helper=scm_helper)
+        self._qtest = None
         self._qtest_path = os.path.join(test_dir, name + "-qtest.sock")
 
     def _base_args(self):