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 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 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/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. 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
