Re: [Qemu-devel] [Qemu-block] [PATCH 09/10] scripts/qemu.py: use a more consistent docstring style

[John Snow]

On 10/04/2018 12:18 PM, Cleber Rosa wrote:
> Signed-off-by: Cleber Rosa <address@hidden>
> ---
>  dtc             |  2 +-
>  scripts/qemu.py | 65 +++++++++++++++++++++++++++++++------------------
>  2 files changed, 42 insertions(+), 25 deletions(-)
> 
> diff --git a/dtc b/dtc
> index 88f18909db..e54388015a 160000
> --- a/dtc
> +++ b/dtc
> @@ -1 +1 @@
> -Subproject commit 88f18909db731a627456f26d779445f84e449536
> +Subproject commit e54388015af1fb4bf04d0bca99caba1074d9cc42
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index f099ce7278..7abe26de69 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -53,9 +53,9 @@ class QEMUMachineAddDeviceError(QEMUMachineError):
>      """
>  
>  class MonitorResponseError(qmp.qmp.QMPError):
> -    '''
> +    """
>      Represents erroneous QMP monitor reply
> -    '''
> +    """

This seems obviously correct, as per the Python Dogma Handbook ...

>      def __init__(self, reply):
>          try:
>              desc = reply["error"]["desc"]
> @@ -66,14 +66,15 @@ class MonitorResponseError(qmp.qmp.QMPError):
>  
>  
>  class QEMUMachine(object):
> -    '''A QEMU VM
> +    """
> +    A QEMU VM
>  
>      Use this object as a context manager to ensure the QEMU process 
> terminates::
>  
>          with VM(binary) as vm:
>              ...
>          # vm is guaranteed to be shut down here
> -    '''
> +    """

As does this,,

>  
>      def __init__(self, binary, args=None, wrapper=None, name=None,
>                   test_dir="/var/tmp", monitor_address=None,
> @@ -135,7 +136,9 @@ class QEMUMachine(object):
>          self._args.append(args)
>  
>      def add_fd(self, fd, fdset, opaque, opts=''):
> -        '''Pass a file descriptor to the VM'''
> +        """
> +        Pass a file descriptor to the VM
> +        """

However, is it established practice among ne'er-do-wells to format
one-line docstrings as three-liners? (And without punctuation to boot --
for shame!)

PEP257 suggests that one-liners are allowed, but doesn't seem to
necessitate their usage. Does this kind of change have any kind of benefit?

>          options = ['fd=%d' % fd,
>                     'set=%d' % fdset,
>                     'opaque=%s' % opaque]
> @@ -168,7 +171,9 @@ class QEMUMachine(object):
>  
>      @staticmethod
>      def _remove_if_exists(path):
> -        '''Remove file object at path if it exists'''
> +        """
> +        Remove file object at path if it exists
> +        """
>          try:
>              os.remove(path)
>          except OSError as exception:
> @@ -271,7 +276,9 @@ class QEMUMachine(object):
>              raise
>  
>      def _launch(self):
> -        '''Launch the VM and establish a QMP connection'''
> +        """
> +        Launch the VM and establish a QMP connection
> +        """
>          devnull = open(os.path.devnull, 'rb')
>          self._pre_launch()
>          self._qemu_full_args = (self._wrapper + [self._binary] +
> @@ -284,14 +291,18 @@ class QEMUMachine(object):
>          self._post_launch()
>  
>      def wait(self):
> -        '''Wait for the VM to power off'''
> +        """
> +        Wait for the VM to power off
> +        """
>          self._popen.wait()
>          self._qmp.close()
>          self._load_io_log()
>          self._post_shutdown()
>  
>      def shutdown(self):
> -        '''Terminate the VM and clean up'''
> +        """
> +        Terminate the VM and clean up
> +        """
>          if self.is_running():
>              try:
>                  self._qmp.cmd('quit')
> @@ -315,7 +326,9 @@ class QEMUMachine(object):
>          self._launched = False
>  
>      def qmp(self, cmd, conv_keys=True, **args):
> -        '''Invoke a QMP command and return the response dict'''
> +        """
> +        Invoke a QMP command and return the response dict
> +        """
>          qmp_args = dict()
>          for key, value in args.items():
>              if conv_keys:
> @@ -326,11 +339,11 @@ class QEMUMachine(object):
>          return self._qmp.cmd(cmd, args=qmp_args)
>  
>      def command(self, cmd, conv_keys=True, **args):
> -        '''
> +        """
>          Invoke a QMP command.
>          On success return the response dict.
>          On failure raise an exception.
> -        '''
> +        """
>          reply = self.qmp(cmd, conv_keys, **args)
>          if reply is None:
>              raise qmp.qmp.QMPError("Monitor is closed")
> @@ -339,13 +352,17 @@ class QEMUMachine(object):
>          return reply["return"]
>  
>      def get_qmp_event(self, wait=False):
> -        '''Poll for one queued QMP events and return it'''
> +        """
> +        Poll for one queued QMP events and return it
> +        """
>          if len(self._events) > 0:
>              return self._events.pop(0)
>          return self._qmp.pull_event(wait=wait)
>  
>      def get_qmp_events(self, wait=False):
> -        '''Poll for queued QMP events and return a list of dicts'''
> +        """
> +        Poll for queued QMP events and return a list of dicts
> +        """
>          events = self._qmp.get_events(wait=wait)
>          events.extend(self._events)
>          del self._events[:]
> @@ -353,7 +370,7 @@ class QEMUMachine(object):
>          return events
>  
>      def event_wait(self, name, timeout=60.0, match=None):
> -        '''
> +        """
>          Wait for specified timeout on named event in QMP; optionally filter
>          results by match.
>  
> @@ -361,7 +378,7 @@ class QEMUMachine(object):
>          branch processing on match's value None
>             {"foo": {"bar": 1}} matches {"foo": None}
>             {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
> -        '''
> +        """
>          def event_match(event, match=None):
>              if match is None:
>                  return True
> @@ -394,29 +411,29 @@ class QEMUMachine(object):
>          return None
>  
>      def get_log(self):
> -        '''
> +        """
>          After self.shutdown or failed qemu execution, this returns the output
>          of the qemu process.
> -        '''
> +        """
>          return self._iolog
>  
>      def add_args(self, *args):
> -        '''
> +        """
>          Adds to the list of extra arguments to be given to the QEMU binary
> -        '''
> +        """
>          self._args.extend(args)
>  
>      def set_machine(self, machine_type):
> -        '''
> +        """
>          Sets the machine type
>  
>          If set, the machine type will be added to the base arguments
>          of the resulting QEMU command line.
> -        '''
> +        """
>          self._machine = machine_type
>  
>      def set_console(self, device_type=None):
> -        '''
> +        """
>          Sets the device type for a console device
>  
>          If set, the console device and a backing character device will
> @@ -434,7 +451,7 @@ class QEMUMachine(object):
>          @param device_type: the device type, such as "isa-serial"
>          @raises: QEMUMachineAddDeviceError if the device type is not given
>                   and can not be determined.
> -        '''
> +        """
>          if device_type is None:
>              if self._machine is None:
>                  raise QEMUMachineAddDeviceError("Can not add a console 
> device:"
>