search

MaddieM4 / EJTP-lib-python

Encrypted JSON Transport Protocol library for Python 2.6-3.3.
GNU Lesser General Public License v3.0
23 stars 10 forks source link

Anti-py25 Pogrom - getting rid of crazy hacks used for Python 2.5 support! ($25) #109

Closed MaddieM4 closed 11 years ago

MaddieM4 commented 11 years ago

First of all, let's build up a good list of the things we can improve/hacks that were necessary for Python 2.5 support but made us weep:

These problems can be found in the following commits:

iurisilvio commented 11 years ago

Remove all from __future__ import with_statement.

MoritzS commented 11 years ago
iurisilvio commented 11 years ago

Probably a grep in commit messages will give you some results blaming py25.

MoritzS commented 11 years ago

Yeah, almost all of these commits are ones adding compatibility for py2.5.

$ git log --grep 2.5
commit 35e685368369a710053b485467f601b8c9a00f7d
Merge: 249d602 5e7e5ae
Author: Moritz Sichert <moritz.sichert@googlemail.com>
Date:   Wed Feb 20 23:42:13 2013 +0100

    Merge all changes but the ones for py2.5 from Philip

commit 4841925597499351ab1552faaf98b02046bb617a
Author: Moritz Sichert <moritz.sichert@googlemail.com>
Date:   Mon Feb 18 12:42:44 2013 +0000

    Made py2and3 compatible with py2.5

commit e45371608527f572e1f5d565be0556db15ef61c6
Author: Iuri de Silvio <iurisilvio@gmail.com>
Date:   Sun Feb 10 14:32:06 2013 -0200

    Cast tuple to list only in python 2.5

commit f9c2d9f30d4f7da83f0bc3e7bee0b5873f815c81
Author: Iuri de Silvio <iurisilvio@gmail.com>
Date:   Sun Feb 10 14:23:38 2013 -0200

    Remove socket.create_connection to support Python 2.5

commit f5e7ce74d8ffb94556ff1f9818f9509518a0bea3
Author: Iuri de Silvio <iurisilvio@gmail.com>
Date:   Sun Feb 10 14:20:28 2013 -0200

    Add some imports to make it compatible with Python 2.5

commit fd25477ec00c7f22290f88635cdc8c124ab0201d
Author: Iuri de Silvio <iurisilvio@gmail.com>
Date:   Sun Feb 10 14:18:51 2013 -0200

    Change notify_all() to notifyAll() to work with Python 2.5

commit c82b57f375c92eb4bffecab46cca8fbcba1149a4
Author: Iuri de Silvio <iurisilvio@gmail.com>
Date:   Sun Feb 10 14:17:53 2013 -0200

    Python 2.5 does not have @prop.setter decorator

commit 0ab9dc5e2f05e581bd7d57f9a25aa44fc04e3a2b
Author: Iuri de Silvio <iurisilvio@gmail.com>
Date:   Sun Feb 10 14:13:45 2013 -0200

    Python 2.5 does not have class decorators

commit 1fb502e9cb1a267963698c11593cf12f11f9557a
Author: Iuri de Silvio <iurisilvio@gmail.com>
Date:   Mon Feb 4 22:25:31 2013 -0200

    Support Python 2.5, 2.6, 3.1 and 3.3 in tox (with tests broken)

commit 691a6bd6f2702de045b274473cc0cab6cec5df8c
Author: Iuri de Silvio <iurisilvio@gmail.com>
Date:   Mon Feb 4 08:35:00 2013 -0200

    Using unittest2 with 2.4, 2.5, 2.6, 3.0 and 3.1
MaddieM4 commented 11 years ago

I've updated the list at the top based on conversation/the list of commits, and I'll put a bounty on this shortly.

MaddieM4 commented 11 years ago

Bounty is $25.

http://www.freedomsponsors.org/core/issue/208/anti-py25-pogrom-getting-rid-of-crazy-hacks-used-for-python-25-support

(Copied from acceptance criteria)

Go through the listed commits and problems, try to find all instances of that in the codebase, and revert these to the cleanest/simplest alternative. If you find other issues during this process, bring them up in discussion so we can decide if they apply to the scope of this cleanup.

MaddieM4 commented 11 years ago

Bumped bounty up to $25 because it felt appropriate.

iurisilvio commented 11 years ago

I'm work on that.

iurisilvio commented 11 years ago

I will not replace sys.version_info[0] with sys.version_info.major. It is not available in Python 2.6.

iurisilvio commented 11 years ago

It is not easy to remove ejtp.util.compat.json because I can't import default json module in ejtp.frame package.

iurisilvio commented 11 years ago

It is not one, but I pushed something to my remote repository: https://github.com/iurisilvio/EJTP-lib-python/tree/remove_py25

iurisilvio commented 11 years ago

I did almost all changes.

Almost all ejtp.util.compat things are used only in tests module. Only the ejtp.util.compat.json is necessary to ejtp code, but it can be removed.

I propose to change ejtp.frame.json module name, because it creates a hard problem to solve, I can't import json in frame module.

MaddieM4 commented 11 years ago

I swear, we figured out how to consistently import from standard modules in the face of naming conflicts. And we've used it before. I just gotta find where we did it...

iurisilvio commented 11 years ago

We can remove the module from sys.path and readd it later. It is an awful hack, people just say "do not use a built-in name".

nandub commented 11 years ago

Camp.. That was on the python-libcps when I created the redis backend. Look for the __import__ at the top.

https://github.com/campadrenalin/python-libcps/commit/6d5fdb226214ff5bf89c18923e6ab502812e85b1

MaddieM4 commented 11 years ago

@nandub Yup, there it is! No wonder I couldn't find it in EJTP.

@iurisilvio So yeah, there is a non-terrible way to do that kind of import. Go ahead and use that.

iurisilvio commented 11 years ago

Thanks! Now, I removed json from compat module.

iurisilvio commented 11 years ago

I guess it is ready for merge.

MaddieM4 commented 11 years ago

Awesome. I'll review and merge it later - we're having company over, so I'm in the middle of cleaning the bathroom right now :) On Mar 1, 2013 6:30 PM, "Iuri de Silvio" notifications@github.com wrote:

I guess it is ready for merge.

— Reply to this email directly or view it on GitHubhttps://github.com/campadrenalin/EJTP-lib-python/issues/109#issuecomment-14321446 .

MaddieM4 commented 11 years ago

Looks great. Merging now. If we run into any more stuff later, we'll deal with it then, but I think we're generally good to go.