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.

Sign up for GitHub

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

Jump to bottom

v0.5.0 prep - READY FOR MERGE #21

Merged
merged 12 commits into from Nov 23, 2013
Merged

v0.5.0 prep - READY FOR MERGE #21

merged 12 commits into from Nov 23, 2013

Conversation

joncotton
Copy link
Contributor

Updates and fixes necessary for an official release.

A previous PR (one of mine) broke testing in Python 2.6 and the Readme needed a refresh. This also officially drops Python 3.1 support.

Other new things

  • setup.py test is available (and used by TravisCI)
  • setup.py uses 2to3 conversion during the build process for Python 3
  • test coverage at 100% (for all tested versions of Python)
  • mock is used in tests
  • minor fixes and improvements

@joncotton
Fix testing in Python 2.6
59048db
@joncotton
Drop Python 3.1 support.
2931368 
I'm not sure this *was* working in Python 3.1, but TravisCI isn't testing it and Tox coughs up an `ImportError: No module named ssl_match_hostname`.  We can look into this more, but supporting 3.1 probably isn't worth it.
@joncotton
close our HTTP request connections
3369502 
Good practice and Python 3.2/3.3 had this to say, `embedly\client.py:152: ResourceWarning: unclosed <socket.socket object, ...>`
@joncotton
Tidying. Additional gitignore lines. Don't need the conditional `simp…
9c7ba5e 
…lejson` because we don't support Python 2.5.
@joncotton
Update Readme
1fedeb4 
Mostly this updates the `Url` model documentation to reflect its more pythonic nature as a true dictionary.
Also update a couple links, don't use `sudo` and fix the RST formatting for a few code blocks.
@joncotton
Copy link
Contributor Author

I'm in the middle of increasing test coverage. Once that's ready I think 0.5.0 is done. Thoughts?

@screeley
Copy link
Contributor

Looks great.

I do believe that we need to make this work on Python 3, but a target of 3.3 would be nice.

Thanks for all your work here.

@joncotton
Copy link
Contributor Author

Python 3.2 and 3.3 are easy. I mean you guys already did that work =)

@screeley
Copy link
Contributor

Nice!

@joncotton
Remove a working but unnecessary recursive call.
24db8ef
@joncotton
Fix an issue when calling get_services() in Python 3.
131cfb3 
(This now matches the response decoding already used in `_get()`. It was never noticed because tests didn't cover it.)
@joncotton
Increase test coverage. Reorder imports so that Python 3 is first.
e1f11b4
@joncotton
Tests - add mocking to test get_services()
4de59b7 
This may be overkill but it does increase test coverage and if we develop more fine-grained tests for `_get()` in the future, mock will help.
@joncotton
Use 2to3 automatically during the build process.
b99d01b 
It doesn't make a ton of changes in truth, but it's a recommended step on the conversion path. It does handle the `urllib` path though!
@joncotton
TravisCI - let's see if it will use setup.py test and thus install …
b9292c4 
…the test dependencies
@joncotton
Python 3 - properly handle __str__ vs __unicode__ .
e692ed8 
Also test for it, which increases test coverage to 100%.
@joncotton
Copy link
Contributor Author

More stuff, namely test coverage is at 100% and I took a hard look at Python 3. I believe some things were previously broken under Python 3 but escaped notice because the test coverage was incomplete.

2to3 tidied some things like removing the from __future__ imports and the urllib import.

__str__ and __unicode__ were harder nuts to crack and this is what I believe would have broken. Running from this commit, 3593925:

(py33) > python
Python 3.3.3 (v3.3.3:c3896275c0f6, Nov 18 2013, 21:18:40)
>>> from embedly.models import Url
>>> unistr = 'I\xf1t\xebrn\xe2ti\xf4n\xe0liz\xe6tion'
>>> print(unistr)
Iñtërnâtiônàlizætion
>>> u = Url(method=unistr)
>>> print(u)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __str__ returned non-string (type bytes)

That certainly isn't an all encompassing scenario, but it's what I expected. So all that's solved and there's a test for it.

@joncotton
Copy link
Contributor Author

I'm declaring this done. Would love another set of eyes and tests though.

@screeley
Copy link
Contributor

It looks good to me, merge away!

joncotton added a commit to joncotton/embedly-python that referenced this pull request Nov 23, 2013
@joncotton
Merge branch 'v0.5.0-prep' and bump version to 0.5.0. Closes PR embed…
6242333 
…ly#21
@joncotton joncotton merged commit e692ed8 into embedly:master Nov 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@joncotton @screeley