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

[Aleš Mareček]

Greetings!
Martin, thanks for your review and comments!
I changed the name of the patch and setup my git variables properly. I also re-tested it and got all passed. I'm sending a new patch that is attached.

----- Original Message -----
> From: "Martin Basti" <mbasti at redhat.com>
> To: "Aleš Mareček" <amarecek at redhat.com>, freeipa-devel at redhat.com
> Sent: Tuesday, March 24, 2015 4:39:21 PM
> Subject: Re: [Freeipa-devel] [PATCH] 0001 ipatests: SOA record Maintenance tests
> 
> 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}),
> +    ]
> 

Done.

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

Because of some updates needs, I didn't want to break anything existing thus I created new.

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

Done.
$ git am freeipa-alich-0001-2-Ipatests-DNS-SOA-Record-Maintenance.patch
Applying: Ipatests DNS SOA Record Maintenance
$

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

Done, only 1 line persisted that I didn't want to break:
zone6_unresolvable_ns_relative_dnsname = DNSName(zone6_unresolvable_ns_relative)

> 
> Otherwise test works as expected.
> 
> Martin^2
> 
> --
> Martin Basti
> 
> 

Thanks!
 - alich -
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-alich-0001-2-Ipatests-DNS-SOA-Record-Maintenance.patch
Type: text/x-patch
Size: 32974 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150327/22a103cb/attachment.bin>