[Freeipa-devel] [PATCHES] 0691-0695 Modernization

[Tomas Babej]

On 08/12/2015 06:16 PM, Tomas Babej wrote:
> 
> 
> On 08/12/2015 06:12 PM, Christian Heimes wrote:
>> On 2015-08-12 18:10, Tomas Babej wrote:
>>>
>>>
>>> On 08/10/2015 05:39 PM, Petr Viktorin wrote:
>>>> On 08/03/2015 11:07 AM, Christian Heimes wrote:
>>>>> On 2015-07-31 19:14, Petr Viktorin wrote:
>>>>>> Hello,
>>>>>> Here is a batch of mostly mechanical changes: removing deprecated
>>>>>> features to prepare for Python 3.
>>>>>
>>>>> Out of curiosity, what tool did you use for patch 695-absolute-imports?
>>>>> Python-modernize adds from __future__ import absolute_imports and
>>>>> changes imports to explicit relative imports.
>>>>
>>>> I used modernize to find all the occurences, and fixed imports by hand.
>>>> Most of IPA uses absolute imports, as recommended by PEP 8.
>>>>
>>>>> In patch 693 you have removed test cases for CIDict.has_key(), but
>>>>> CIDict still provides the function. You should either keep the tests
>>>>> around or remove has_key() from CIDict.
>>>>
>>>> I haven't removed them: "test_haskey" is only skipped under Python 3. I
>>>> assumed that's enough to verify that `has_key` works well (i.e. the same
>>>> as `in`), so in the other tests I do use `in` instead.
>>>>
>>>> I'm attaching updated patches, under Python 3 they remove CIDict.has_key
>>>> a bit more formally. They're also rebased.
>>>>
>>>>> The rest looks good to me, but I haven't studied every change
>>>>> thoroughly. It's just too much.
>>>>
>>>> Anything I can do to help?
>>>
>>> Let's not sit on this for too long, it will a pain to rebase. I went
>>> through the gargatuan patches manually and did not discover any issues.
>>>
>>> Additionally, the patchset introduces no new unit-test failures.
>>>
>>> So I am inclined to ACK it, unless Christian has any objections.
>>
>> I've skimmed over the patches and didn't find any issues, too.
>>
>> pylint --py3k is going to complain about missing from __future__ import
>> absolute_import lines. We can add them later, though.
>>
>> Christian
>>
>>
> 
> Either that, or we can simply ignore no-absolute-import (W1618).
> 
> Thus ACK for the patchset.
> 
> Tomas
> 

Pushed to master: 5435a8a32a2e88675e84d22d6f9b97e67f6f5264