Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python 3.3 support #10

Merged
merged 14 commits into from
Apr 24, 2014
Merged

Python 3.3 support #10

merged 14 commits into from
Apr 24, 2014

Conversation

itamarst
Copy link
Owner

Fixes #1, in theory.

from eliot import tai64n

if PY3:
from eliot import _py3json as json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the convention was to use explicit relative imports.

pyjson = json
else:
try:
# ujson is pretty crappy... but much faster than built-in json module, at
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really related to this branch specifically, but I guess it might be nice not to say things like "ujson is pretty crappy" (now that this comment is public :). Perhaps it even would have been good if this comment had been more specific to begin with because right now I'm trying to remember what we don't like about ujson. Maybe it skips a bunch of error handling and generates garbage output for garbage input?

@exarkun
Copy link
Collaborator

exarkun commented Apr 24, 2014

Only fairly minor comments from me... I'm just relying on travis to know if this actually makes it work or not (and travis says it does). I think the comment about ujson and the skip messages are worth fixing. The rest is up to you. Address and merge at your discretion. Thanks!

itamarst added a commit that referenced this pull request Apr 24, 2014
@itamarst itamarst merged commit aa550bd into master Apr 24, 2014
@itamarst itamarst deleted the python3-1 branch April 24, 2014 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python 3.3+ support
3 participants