On 01.07.2016 15:51, Florence Blanc-Renaud wrote:
On 06/21/2016 01:51 PM, Martin Basti wrote:


On 21.06.2016 08:38, Florence Blanc-Renaud wrote:
On 06/20/2016 07:08 PM, Martin Basti wrote:


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 <55042ba...@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 <55042ba...@sstebrno.eu>
To: pspa...@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 :)


Patch 0003 is OK, ACK for this one.
Flo.

Patch 0003: Pushed to master: 94909d21dbf033cbe34089782c430ec25b9ad0bc

Hi,

please find my comments on the remaining patches.

- Patch 0005 must be rebased because of changes in ipatests/test_integration/tasks.py
the patch can also modify pylintrc (remove pointless-statement)

- Patch 0006:
no need to rename the items in "for e in ...", renaming the Exception as exc should be enough

- Patch 0007:
pylintrc should remove old-style-class instead of bad-classmethod-argument

- Patch 0008:
this one should remove bad-classmethod-argument in pylintrc

- Patch 0009:
ok

- Patch 0010:
In the __bind method(self, obj_type), cls variable is already used thus replacing self with cls can be done only if cls is also renamed into something else.

Flo.


Please follow new changes in this PR https://github.com/freeipa/freeipa/pull/97
It will be easier to review.

Martin^2

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to