On 24/03/15 15:06, Aleš Mareček wrote:
This is my very first patch, ticket#4746.

Have a nice day!
  - alich -

Thank you for the patch. Just nitpicks:

+    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}),
+    ]

I'm fine with zone6b, but was there any reason to create zone6b, instead of reusing zone 1 or 2 or 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.

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 Basti

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

Reply via email to