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

Add support for Python3 #21

Merged
merged 8 commits into from
Sep 20, 2018
Merged

Add support for Python3 #21

merged 8 commits into from
Sep 20, 2018

Conversation

pbauer
Copy link
Member

@pbauer pbauer commented May 5, 2017

The tests only run together with plone/plone.rfc822#4
We tried not to use six but are open to any suggestions.

@pbauer
Copy link
Member Author

pbauer commented May 5, 2017

@mister-roboto you are lying, you evil robot overlord. @dhavlik signed the CA and sent it in.

@gforcada
Copy link
Member

gforcada commented May 6, 2017

(in behalf of mr.roboto)

@pbauer no, mr.roboto is not lying! See it by yourself:

https://api.github.com/repos/plone/plone.supermodel/commits/e2aa2add70c4e01e61df5acebd019461b5f796d0

https://api.github.com/repos/plone/plone.supermodel/commits/d274c5c3893af0f4e3dbf7e13ef2a7f2f88c07cb

The two commits made by @dhavlik are not using any email (as mr.roboto complains) that @dhavlik has on github. That should be updated.

You can clearly recognize that seeing that there is the octopus icon rather than @dhavlik avatar on the commits list: https://github.com/plone/plone.supermodel/pull/21/commits

@pbauer
Copy link
Member Author

pbauer commented May 10, 2017

true and wise, oh great robot!

@gforcada
Copy link
Member

@pbauer if you merge this or other pull requests adding support to python 3, would you mind creating an issue on buildout.coredev so that the py3 buildout configuration for the jenkins job gets updated as well?

@davilima6
Copy link
Member

davilima6 commented Jan 30, 2018

Current failure is related to indentation.

Also needs rebasing master.

@davilima6 davilima6 force-pushed the python3 branch 4 times, most recently from b78edf9 to 20ab7f7 Compare January 30, 2018 11:45
@davilima6
Copy link
Member

davilima6 commented Jan 30, 2018

@pbauer, @mauritsvanrees: tests pass for Python 2 but still not for 3.

There's one commented line that must be removed in plone/supermodel/__init__.py but I think it's time for a review before I go on, in order to check I'm going in the right direction, or forcing some serialisation step too early (I suspect that's what I might be doing wrong for Py3 tests).

Copy link
Member

@mauritsvanrees mauritsvanrees left a comment

Choose a reason for hiding this comment

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

I haven't really looked at this package before, so I don't feel I can really review this. I don't even know how to run the Python3 tests. Since this package has so few dependencies, a small buildout and Travis would be nice, but don't let that hinder this PR.
From a superfluous glance, the PR seems okay.

@jensens
Copy link
Member

jensens commented Sep 20, 2018

I fixed @dhavlik email address (was something with .local) by interactive rebase.

For future commits, please use the email addresses known by Github (you can add more at your profile page)

@jensens jensens merged commit b6af529 into master Sep 20, 2018
@jensens jensens deleted the python3 branch September 20, 2018 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants