Re: [PATCH 03/20] python/machine.py: reorder __init__

[John Snow <jsnow@redhat.com>]

On 10/7/20 5:43 AM, Kevin Wolf wrote:
> Am 07.10.2020 um 01:58 hat John Snow geschrieben:
>> Put the init arg handling all at the top, and mostly in order (deviating
>> when one is dependent on another), and put what is effectively runtime
>> state declaration at the bottom.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   python/qemu/machine.py | 44 ++++++++++++++++++++++++------------------
>>   1 file changed, 25 insertions(+), 19 deletions(-)
>>
>> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
>> index 3017ec072df..71fe58be050 100644
>> --- a/python/qemu/machine.py
>> +++ b/python/qemu/machine.py
>> @@ -84,42 +84,54 @@ def __init__(self, binary, args=None, wrapper=None, name=None,
>>           @param monitor_address: address for QMP monitor
>>           @param socket_scm_helper: helper program, required for send_fd_scm()
>>           @param sock_dir: where to create socket (overrides test_dir for sock)
>> -        @param console_log: (optional) path to console log file
>>           @param drain_console: (optional) True to drain console socket to buffer
>> +        @param console_log: (optional) path to console log file
>>           @note: Qemu process is not started until launch() is used.
>>           '''
>> +        # Direct user configuration
>> +
>> +        self._binary = binary
>> +
>>           if args is None:
>>               args = []
>> +        # Copy mutable input: we will be modifying our copy
>> +        self._args = list(args)
>> +
>>           if wrapper is None:
>>               wrapper = []
>> -        if name is None:
>> -            name = "qemu-%d" % os.getpid()
>> -        if sock_dir is None:
>> -            sock_dir = test_dir
>> -        self._name = name
>> +        self._wrapper = wrapper
>> +
>> +        self._name = name or "qemu-%d" % os.getpid()
>> +        self._test_dir = test_dir
>> +        self._sock_dir = sock_dir or self._test_dir
>> +        self._socket_scm_helper = socket_scm_helper
> 
> Interesting that you use a shortcut with 'or' for name and sock_dir,
> but you don't have 'wraper or []' and 'args or []' above.
> 
> It's not wrong, of course, but if you have to respin for another reason,
> maybe an inconsistency to address.
> 

This winds up being because ... I delete those lines of code later. I 
very often have this problem where I clean up a bunch of stuff, and then 
split out that giant commit into a series.

Sometimes that causes stuff like this.

> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> 

Thanks!