[Freeipa-devel] [PATCH] 0001 ipatests: SOA record Maintenance tests

Martin Basti mbasti at redhat.com
Tue Mar 24 15:39:21 UTC 2015


On 24/03/15 15:06, Aleš Mareček wrote:
> Greetings!
> This is my very first patch, ticket#4746.
>
> Have a nice day!
>   - alich -
>
>
Thank you for the patch. Just nitpicks:

1)
+    cleanup_commands = [
+        ('dnszone_del', [zone6], {'continue': True}),
+        ('dnszone_del', [zone6b], {'continue': True}),
+    ]

would be better do it in this way, continue option will to try remove 
all zones:
+    cleanup_commands = [
+        ('dnszone_del', [zone6, zone6b], {'continue': True}),
+    ]

2)
I'm fine with zone6b, but was there any reason to create zone6b, instead 
of reusing zone 1 or 2 or 3?

3)
Please fix whitespace errors.
$ git am 
freeipa-alich-0001-ipatests-added-tests-for-SOA-record-Maintenance.patch
Applying: ipatests - added tests for SOA record Maintenance
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:482: trailing 
whitespace.

/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:758: new blank 
line at EOF.
+
warning: 2 lines add whitespace errors.

4)
I know the dns plugin tests are so far from PEP8, but try to keep PEP8 
in new code

Otherwise test works as expected.

Martin^2

-- 
Martin Basti

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150324/97b8b9c5/attachment.htm>


More information about the Freeipa-devel mailing list