[Freeipa-devel] [PATCH 0473-0476, 0478-0482]DNS Locations: Prologue

[Petr Spacek]

On 3.6.2016 12:51, Martin Basti wrote:
> 
> 
> On 03.06.2016 08:53, Petr Spacek wrote:
>> On 2.6.2016 17:53, Martin Basti wrote:
>>> <snip>
>>>>>>>> Typo - redundant ' ' at the end.
>>>>>>>>
>>>>>>>>
>>>>>>>> Conditional NACK, warnings mentioned in
>>>>>>>> http://www.freeipa.org/page/V4/DNS_Location_Mechanism#CLI
>>>>>>>> are not there.
>>>>>>>>
>>>>>>>> I'm open to changing this to ACK if you open a separate ticket for this
>>>>>>>> omission so we do not forget to add them later on.
>>> I forgot to add, this will be in next batch of patches (you may see that there
>>> are not marked DNS servers in output of location show), I do not see reason to
>>> open ticket when the current one is not finished.
>>>
>>>>>>> +1
>>>>>>>
>>>> Done
>>>>
>>>>>>> Patch 480:
>>>>>>>
>>>>>>> 1) The code in location_show.execute() looks like it could be moved to
>>>>>>> location_show.post_callback()
>>>>>>>
>>>> I had to add it to execute because I modifies result entry not just
>>>> entry_attrs
>>>>
>>>>>>> 2) Before calling super().output_for_cli(), pop 'servers' from result, so
>>>>>>> that
>>>>>>> it is not displayed with --all.
>>>>>>>
>>>>>>>
>>>> Done
>>>>
>>>>>>> Patch 481:
>>>>>>>
>>>>>>> 1) Could we rename --force to --nonempty (or something better)? I would
>>>>>>> like
>>>>>>> to reserve --force for "ignore NotFound when deleting the entry", which
>>>>>>> is not
>>>>>>> the case here.
>>>>>> IMHO option is unnecessary. Just delete the location (and unset location
>>>>>> from
>>>>>> all member servers). The design does not contain --force anyway :-)
>>>>> OK, that's even better :-)
>>>>>
>>>> Done
>>>>
>>>> Updated patches attached
>> I had to add top object class to the plugin and tests to make tests pass.
>> Patch is attached.
>>
>> CondACK: Fix this before pushing somehow.
>>
> 
> Updated and heavily rebased patches attached.

ACK

-- 
Petr^2 Spacek