GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — v0.5.0 prep - READY FOR MERGE by joncotton · Pull Request #21 · embedly/embedly-python

Conversation

@joncotton

@joncotton joncotton commented Nov 15, 2013

Copy link
Copy Markdown
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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
joncotton added 5 commits November 14, 2013 21:16
@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

joncotton commented Nov 16, 2013

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@screeley

screeley commented Nov 18, 2013

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@joncotton

joncotton commented Nov 18, 2013

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@screeley

screeley commented Nov 18, 2013

Copy link
Copy Markdown
Contributor

Nice!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

joncotton added 7 commits November 21, 2013 14:46
@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

joncotton commented Nov 22, 2013

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@joncotton

joncotton commented Nov 22, 2013

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@screeley

screeley commented Nov 22, 2013

Copy link
Copy Markdown
Contributor

It looks good to me, merge away!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@joncotton @screeley
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.