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

Remove all traces of Products.CMFQuickInstaller #1267

Merged
merged 1 commit into from
Nov 22, 2021

Conversation

jensens
Copy link
Member

@jensens jensens commented Nov 20, 2021

For some reason I got an Products.CMFQuickInstaller import error in Plone 6. Maybe because its gone? Question is why it worked in past? Did it?

Anyway, this fixes the problem.

@mister-roboto
Copy link

@jensens thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

@jensens jensens force-pushed the fix-missing-quickinstaller branch from 6ed4add to 84c05dd Compare November 20, 2021 18:42
@jensens
Copy link
Member Author

jensens commented Nov 20, 2021

@jenkins-plone-org please run jobs

@@ -2,16 +2,22 @@
from Products.CMFCore.utils import getToolByName
from Products.CMFPlone import PloneMessageFactory as _
from Products.CMFPlone.interfaces import INonInstallable
from Products.CMFQuickInstallerTool.interfaces import (
Copy link
Member

Choose a reason for hiding this comment

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

Uhm, this is strange, the iface was moved to:

from Products.CMFPlone.interfaces import INonInstallable

And we should have aliases for that:

https://github.com/plone/plone.app.upgrade/blob/ef4aef834c561f7a634e819571b7c3bfb48cf314/plone/app/upgrade/__init__.py#L185

Don't you have plone.app.upgrade?

I think the proper fix is to replace the import given that the alias is there since 5.1:
https://github.com/plone/Products.CMFPlone/blob/5.1.x/Products/CMFPlone/interfaces/__init__.py#L38
and this package supports Plone 5.2 only.

Also the towncrier entry should be updated accordingly

Copy link
Member Author

Choose a reason for hiding this comment

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

No I do only depend on Products.CMFPlone. Which is totally fine. plone.app.upgrade in only installed in a single service instance. The cluster runs usually without.

Copy link
Member Author

@jensens jensens Nov 20, 2021

Choose a reason for hiding this comment

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

I think the proper fix is to replace the import given that the alias is there since 5.1

Right, this the current master does not support 5.1 anyway. Removal is probably the best.

@jensens jensens force-pushed the fix-missing-quickinstaller branch from 84c05dd to afc2301 Compare November 20, 2021 22:38
@jensens jensens changed the title Make quickinstaller optional Remove all traces of Products.CMFQuickInstaller Nov 20, 2021
@jensens jensens force-pushed the fix-missing-quickinstaller branch from afc2301 to 6900c94 Compare November 20, 2021 22:40
@jensens
Copy link
Member Author

jensens commented Nov 20, 2021

@jenkins-plone-org please run jobs

@jensens jensens merged commit a2d9937 into master Nov 22, 2021
@jensens jensens deleted the fix-missing-quickinstaller branch November 22, 2021 08:33
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.

3 participants