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

Port to Python 3 #49

Merged
merged 34 commits into from
Oct 17, 2018
Merged

Port to Python 3 #49

merged 34 commits into from
Oct 17, 2018

Conversation

loechel
Copy link
Member

@loechel loechel commented Apr 23, 2018

This PR requires that the py3 branch of plone.testing is merged. So It cannot be tested separately against coredev on Jenkins. See plone/buildout.coredev#480 for a test running both py3 branches.

@icemac icemac changed the title Start porting to Python 3 Port to Python 3 May 17, 2018
@icemac icemac requested a review from mauritsvanrees May 17, 2018 16:07
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.

With such a big diff, I can't review details. (Well, I made a few minor comments.) But the general direction seems sane.

"""
setHooks()
site = getSite()

with z2.zopeApp(db, connection, environ) as app:
with getattr(flavour, 'zopeApp')(db, connection, environ) as app:
Copy link
Member

Choose a reason for hiding this comment

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

I would say either use flavour.zopeApp directly, or use getattr and explicitly handle the case that it falls back to None.

CHANGES.rst Outdated
Bug fixes:

- *add item here*
- *none yet*
Copy link
Member

Choose a reason for hiding this comment

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

Minor nitpick: if you keep * add item * here, then plone.releaser can automatically remove it on release, which is what you want here. If you add any other text, then plone.releaser does not touch it.

Set up plone.testing.z2.Startup in ... seconds.
Set up plone.app.testing.layers.PloneFixture in ... seconds.
>>> runner.setup_layer(options, layers.PLONE_FIXTURE, setupLayers) # doctest: +ELLIPSIS
Set up plone.testing.zca.LayerCleanup in ... seconds.
Copy link
Member

Choose a reason for hiding this comment

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

Is the extra two space indentation intentional?

jensens and others added 5 commits June 26, 2018 23:49
@pbauer pbauer mentioned this pull request Sep 24, 2018
9 tasks
@pbauer pbauer merged commit 11c2d9f into master Oct 17, 2018
@jensens jensens deleted the py3 branch October 17, 2018 10:01
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