Re: [Freeipa-devel] [PATCHES] 0691-0695 Modernization
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 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCHES] 0691-0695 Modernization
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 signature.asc Description: OpenPGP digital signature -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCHES] 0691-0695 Modernization
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 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCHES] 0691-0695 Modernization
On 07/31/2015 11:14 PM, Simo Sorce wrote: On Fri, 2015-07-31 at 19:14 +0200, Petr Viktorin wrote: Hello, Here is a batch of mostly mechanical changes: removing deprecated features to prepare for Python 3. Do we have accompanying lint (or similar) tests that will prevent new patches from reintroducing py3 incompatible syntax ? Not yet. These patches are just first steps. Parts of IPA is still not compatible with Python 3 (even syntactically). So, linter would currently need an extensive blacklist/whitelist to be effective. -- Petr Viktorin -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCHES] 0691-0695 Modernization
On 2015-07-31 23:14, Simo Sorce wrote: On Fri, 2015-07-31 at 19:14 +0200, Petr Viktorin wrote: Hello, Here is a batch of mostly mechanical changes: removing deprecated features to prepare for Python 3. Do we have accompanying lint (or similar) tests that will prevent new patches from reintroducing py3 incompatible syntax ? pylint has a Python 3 porting mode. That should help, see patch 022. Christian signature.asc Description: OpenPGP digital signature -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCHES] 0691-0695 Modernization
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. 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. The rest looks good to me, but I haven't studied every change thoroughly. It's just too much. Christian signature.asc Description: OpenPGP digital signature -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCHES] 0691-0695 Modernization
On Fri, 2015-07-31 at 19:14 +0200, Petr Viktorin wrote: Hello, Here is a batch of mostly mechanical changes: removing deprecated features to prepare for Python 3. Do we have accompanying lint (or similar) tests that will prevent new patches from reintroducing py3 incompatible syntax ? Simo. -- Simo Sorce * Red Hat, Inc * New York -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
