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

[Jan Cholasta]

Dne 27.3.2015 v 16:54 Martin Basti napsal(a):
> On 27/03/15 16:34, Aleš Mareček wrote:
>> 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 -
> Thank you, ACK.
>

Pushed to:
master: ca96ecbf40038d09814f99f19bf47246352dfa0c
ipa-4-1: 8f94ac1e7c24b3bf33c5211d3e327c9a51390fb1

-- 
Jan Cholasta