Hi,

On 29.1.2016 15:49, Martin Basti wrote:


On 29.01.2016 15:49, Stanislav Laznicka wrote:
Reworded the commits so that they better reflect what's going on in those.

On 01/29/2016 02:49 PM, Stanislav Laznicka wrote:
Hello,

I made some patches based on the Coverity report from 18.1.2016.

Cheers,
Standa





NACK, see my previous email

I don't think this deserves 9 patches, 1 would be sufficient enough.


Patch 0013:

1) I think this unreachable return is intentional, as indicated by the comment:

-            #we shouldn't get here
-            return [UNKNOWN_ERROR]

2) How is this dead code?

-    if options.mode == 'validate_pot' or options.mode == 'validate_po':
+    if options.mode in ('validate_pot', 'validate_po'):

-    elif options.mode == 'create_test' or 'test_gettext':
+    elif options.mode in ('create_test', 'test_gettext'):



Patch 0014-0015: LGTM


Patch 0016: The original code is in fact correct.


Patch 0017: This will break Python 3. The two branches are performing the same action, but with different data types.


Patch 0018: LGTM


Patch 0019: IMO the original code is better (None has a __class__ too, you know).


Patch 0020: LGTM


Patch 0021: Please use the original error messages (there are no requests being added to D-Bus, but to certmonger).


Honza

--
Jan Cholasta

--
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