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

[Petr Spacek]

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.

-- 
Petr^2 Spacek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: top_objclass.patch
Type: text/x-patch
Size: 1084 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160603/d999e3d2/attachment.bin>