[Freeipa-devel] [PATCHES] 0688-0689 Remove Editable DN and DN component classes

[Jan Cholasta]

Dne 20.4.2015 v 18:16 Petr Viktorin napsal(a):
> On 04/20/2015 05:19 PM, Jan Cholasta wrote:
>> Dne 20.4.2015 v 17:13 Petr Viktorin napsal(a):
>>> On 04/20/2015 10:24 AM, Jan Cholasta wrote:
>>>> Dne 16.4.2015 v 14:35 Petr Viktorin napsal(a):
>>>>> On 04/16/2015 09:04 AM, Jan Cholasta wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Dne 10.4.2015 v 15:58 Petr Viktorin napsal(a):
>>>>>>> The attached patches remove EditableDN, EditableRDN and EditableAVA.
>>>>>>> They depend on Petr Voborník's patch 811 (performance: faster DN
>>>>>>> implementation).
>>>>>>>
>>>>>>>
>>>>>>> Mutable DNs are not very useful. When creating them it is easier to
>>>>>>> work
>>>>>>> with lists or generators, and needing to change DNs aside from
>>>>>>> operations like `DN(new_rdn, original[1:])` is very rare -- I'd even
>>>>>>> say
>>>>>>> theoretical.
>>>>>>> Mutable DNs are not hashable, so they can't be used as dist keys.
>>>>>>> Storing them as "keys" in other structures (e.g. in a LDAPEntry) is
>>>>>>> dangerous -- it's hard to reason about outside modifications.
>>>>>>>
>>>>>>> The first patch removes the last use of EditableDN. I could be
>>>>>>> convinced
>>>>>>> it's not an improvement in elegance/readability, but I believe
>>>>>>> this is
>>>>>>> the strongest case for EditableDN in IPA, and it doesn't justify
>>>>>>> keeping
>>>>>>> it.
>>>>>>
>>>>>> LGTM, but patch 688 needs to be rebased.
>>>>>
>>>>> Here you go.
>>>>
>>>> Regarding patch 688, it seems we are always replacing the suffix of the
>>>> DN, so I think we can simplify _dn_replace to:
>>>>
>>>>       if not dn.endswith(old):
>>>>           raise ValueError('no replacement made')
>>>>       return DN(*dn[:-len(old)]) + new
>>>>
>>>
>>> Sure, here's a patches with this change.
>>>
>>
>> Thanks, but it looks like you forgot to raise the ValueError.
>>
>
> Ah, sorry for that. Fixed.
>

ACK.

Pushed to master: 2cafb47ed7e4eba354355fa3aed1d8bcbac5fb68

-- 
Jan Cholasta