[Freeipa-devel] [PATCH 0043] Properly handle ipa-replica-install when its zone is not managed by IPA

[Ana Krivokapic]

On 03/29/2013 03:11 PM, Tomas Babej wrote:
> On 03/29/2013 02:15 PM, Ana Krivokapic wrote:
>> On 03/26/2013 04:59 PM, Tomas Babej wrote:
>>> Hi,
>>>
>>> The ipa-replica-install script tries to add replica's A and PTR
>>> records to the master DNS, if master does manage DNS. However,
>>> master need not to manage replica's zone. Properly handle this use
>>> case.
>>>
>>> https://fedorahosted.org/freeipa/ticket/3496
>>>
>>>
>>>
>>> _______________________________________________
>>> Freeipa-devel mailing list
>>> Freeipa-devel at redhat.com
>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>
>> The patch works well and fixes the issue.
>>
>> Just a couple of nitpicks:
>>
>> 1) "However, master need not to manage replica's zone." -- This
>> sentence sounds a little strange to me, but I am not a native speaker
>> so I may be wrong about that.
>
> The phrase should be ok. I assume you're worried about "need not"
> construct, which may sound a bit unusal as opposed to, for example,
> "does not need to".
>
> One could argue that it sounds archaic. However, consider the
> following chart, which clearly proves the opposite:
>
> http://books.google.com/ngrams/chart?content=need%20not%2Cneeds%20not%2Cdoes%20not%20need%20to%2Cdoesn%20'%20t%20need%20to&corpus=0&smoothing=3&year_start=1800&year_end=2000
> <http://books.google.com/ngrams/chart?content=need%20not%2Cneeds%20not%2Cdoes%20not%20need%20to%2Cdoesn%20%27%20t%20need%20to&corpus=0&smoothing=3&year_start=1800&year_end=2000>
>
> For more detailed explanation, see:
>
> http://english.stackexchange.com/questions/29409/why-use-need-not-instead-of-do-not-need-to
>
Actually, the part that sounded weird to me is the "to" that comes after
"need not" in your commit message. Also, the stackexchange link you
provided states: "This /need/ is a *modal verb*: it always requires an
infinitive without /to/;".

Sorry that I wasn't clear about this in my first email.

>>
>> 2) There are three PEP8 501 errors introduced by the patch, but given
>> the recent discussion on this subject, I think it is really up to you
>> if you want to take the time to fix these.
>
> Sure I do. Thanks for the catch. Updated patch attached.
There is still one line with E501:

install/tools/ipa-replica-install:303:80: E501 line too long (80 > 79
characters)

>
>>
>> ACK from the functional perspective.
>>
>> -- 
>> Regards,
>>
>> Ana Krivokapic
>> Associate Software Engineer
>> FreeIPA team
>> Red Hat Inc.
>


-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130329/478aa63f/attachment.htm>