Re: [Qemu-devel] [PATCH v2 09/10] qmp.py: Avoid overriding a builtin object

[Lukáš Doktor]

Dne 25.7.2017 v 21:34 Eduardo Habkost napsal(a):
> On Tue, Jul 25, 2017 at 05:09:50PM +0200, Lukáš Doktor wrote:
>> The "id" is a builtin method to get object's identity and should not be
>> overridden. This might bring some issues in case someone was directly
>> calling "cmd(..., id=id)" but I haven't found such usage on brief search
>> for "cmd\(.*id=".
>>
>> Signed-off-by: Lukáš Doktor <address@hidden>
> 
> I don't see the benefit of the patch, as the function is very
> short and unlikely to ever use the id() builtin.  But I am not
> against it if it will silence code analyzer warnings.
> 
For me the biggest benefit is it decreases the probability of someone copying 
the same "idea" to other pieces of code, because, you know, developers are 
using copy&paste way too often.

Anyway pylint does complain about this issue. I can either ignore it, skip it 
with an informative comment `# use id for backward compatibility pylint: 
disable=W0622` or use this patch to fix it properly. I'm inclined towards 
fixing it (until it poisons other parts of the code), you are in favor of 
ignoring it, so let's see whether someone else has another opinion, otherwise 
I'll remove it in v3.

Lukáš

> 
>> ---
>>  scripts/qmp/qmp.py | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
>> index f2f5a9b..ef12e8a 100644
>> --- a/scripts/qmp/qmp.py
>> +++ b/scripts/qmp/qmp.py
>> @@ -177,19 +177,19 @@ class QEMUMonitorProtocol(object):
>>              print >>sys.stderr, "QMP:<<< %s" % resp
>>          return resp
>>  
>> -    def cmd(self, name, args=None, id=None):
>> +    def cmd(self, name, args=None, cmd_id=None):
>>          """
>>          Build a QMP command and send it to the QMP Monitor.
>>  
>>          @param name: command name (string)
>>          @param args: command arguments (dict)
>> -        @param id: command id (dict, list, string or int)
>> +        @param cmd_id: command id (dict, list, string or int)
>>          """
>>          qmp_cmd = {'execute': name}
>>          if args:
>>              qmp_cmd['arguments'] = args
>> -        if id:
>> -            qmp_cmd['id'] = id
>> +        if cmd_id:
>> +            qmp_cmd['id'] = cmd_id
>>          return self.cmd_obj(qmp_cmd)
>>  
>>      def command(self, cmd, **kwds):
>> -- 
>> 2.9.4
>>
>

signature.asc
Description: OpenPGP digital signature