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

Adapt to changes of MemberData in CMFCore #25

Merged
merged 4 commits into from
Oct 21, 2017
Merged

Adapt to changes of MemberData in CMFCore #25

merged 4 commits into from
Oct 21, 2017

Conversation

pbauer
Copy link
Member

@pbauer pbauer commented Sep 28, 2017

Do not merge!
Fixes #24 for compatability with Zope 4

@pbauer pbauer requested a review from jensens September 28, 2017 09:36
@jensens jensens changed the title WIP: Adapt to changes of MemberData in CMFCore Adapt to changes of MemberData in CMFCore Oct 1, 2017
Copy link
Member

@jensens jensens left a comment

Choose a reason for hiding this comment

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

LGTM (I would remove the FIXME)

@implementer(IManageCapabilities)
class MemberData(BaseMemberData):
# FIXME: MemberData should probably be a more specific adapter
# instead of a subclass.
Copy link
Member

Choose a reason for hiding this comment

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

If the Interface is the same, there is no need to subclass IMember just for the sake of being more specific, so IMO this is perfectly fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the review. I'll remove the the FIXME.

@pbauer
Copy link
Member Author

pbauer commented Oct 9, 2017

I guess we need a legacy-branch when we merge this. Because this will not work in Plone 5.x. Merging can wait untill we have a stable 5.1 release and/or a new 6.0 branch for the coredev.

@jensens
Copy link
Member

jensens commented Oct 9, 2017

Imo: Branch the current master as 5.x, and set it in coredev sources.cfg as the current. Increase major version number here to 6.0.dev0, in setup.py depend on CMFCore >=... and go on.

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.

4 participants