[Freeipa-devel] [PATCH] pylint fixes

[Martin Basti]

On 20.06.2016 19:06, Martin Basti wrote:
>
>
>
> On 20.06.2016 12:00, Florence Blanc-Renaud wrote:
>> On 06/09/2016 05:10 PM, Petr Spacek wrote:
>>> Hello,
>>>
>>> I've received a bunch of pylint fixes produced by upstream contributor who is
>>> not subscribed to the list so I'm resending them here.
>>>
>>> All credit goes to Bárta Jan<55042barta at sstebrno.eu>.
>>>
>>> Flo, if you have time for it I think that it could be a good exercise which
>>> will lead you to various dark corners in IPA :-)
>>>
>>> Petr^2 Spacek
>>>
>>>
>>> -------- Forwarded Message --------
>>> Date: Fri, 3 Jun 2016 14:57:16 +0200
>>> From: Bárta Jan<55042barta at sstebrno.eu>
>>> To:pspacek at redhat.com
>> ___- In the patch 
>> 0002-pylint-fix-simplifiable-if-statement-warnings.patch:_
>>
>> diff --git a/ipatests/test_integration/tasks.py 
>> b/ipatests/test_integration/tasks.py
>> index aebd907..ca2e10f 100644
>> --- a/ipatests/test_integration/tasks.py
>> +++ b/ipatests/test_integration/tasks.py
>> @@ -149,11 +149,7 @@ def host_service_active(host, service):
>>      res = host.run_command(['systemctl', 'is-active', '--quiet', 
>> service],
>>                             raiseonerr=False)
>>
>> -    if res.returncode == 0:
>> -        return True
>> -    else:
>> -        return False
>> -
>> +    return res.returncode
>>
>> should be instead: return res.returncode *== 0* (otherwise the return 
>> type is an int and not a boolean).
>>
>> In the same file:
>> @@ -295,11 +291,7 @@ def 
>> master_authoritative_for_client_domain(master, client):
>>      zone = ".".join(client.hostname.split('.')[1:])
>>      result = master.run_command(["ipa", "dnszone-show", zone],
>>                                  raiseonerr=False)
>> -    if result.returncode == 0:
>> -        return True
>> -    else:
>> -        return False
>> -
>> +    result.returncode == 0
>>
>> should be instead: *return* result.returncode == 0 (otherwise there 
>> is no return statement)
>>
>> diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
>> index 197814c..36b6ba5 100644
>> --- a/ipaserver/plugins/dogtag.py
>> +++ b/ipaserver/plugins/dogtag.py
>> @@ -1689,12 +1689,7 @@ class ra(rabase.rabase):
>>          # Return command result
>>          cmd_result = {}
>>
>> -        if parse_result.get('revoked') == 'yes':
>> -            cmd_result['revoked'] = True
>> -        else:
>> -            cmd_result['revoked'] = False
>> -
>> -        return cmd_result
>> +        cmd_result['revoked'] = parse_result.get('revoked')
>>
>> Should be instead: cmd_result['revoked'] = 
>> parse_result.get('revoked') *== 'yes'* (otherwise the type is a 
>> string and not a boolean)
>>
>> _- in the patch 00__04-pylint-fix-unneeded-not.patch_
>>
>> @@ -632,7 +632,7 @@ class host_add(LDAPCreate):
>>                      options['ip_address'],
>>                      check_forward=True,
>>                      check_reverse=check_reverse)
>> -        if not options.get('force', False) and not 'ip_address' in 
>> options:
>> +        if options.get('force', False) and 'ip_address' not in options:
>>
>> Should be instead: if *not* options.get('force', False) and 
>> 'ip_address' not in options:
>> because of operators precedence
>>
>> I will review patches 0005 to 0010 later today.
>> Flo.
>>
>>
>
> How about patches 1, and 3? Because patches are independent, we can 
> separately ACK them and push them.
>
> Martin^2
>
>

Sorry, I just noticed that there is no patch 1 :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160620/bcdc0a3e/attachment.htm>