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 some registry settings for datetime options #108

Closed
wants to merge 4 commits into from

Conversation

petschki
Copy link
Member

No description provided.

petschki referenced this pull request Sep 14, 2015
…ead of 30. Aligns with old Plone behavior and Plone 5.
@@ -38,7 +38,8 @@
# nedded because users vocabulary was added here
'plone.app.vocabularies>=2.1.12dev',
# needed for compatibility with jQuery 1.9+
'Products.CMFPlone>=4.3.4'
'Products.CMFPlone>=4.3.4',
'plone.app.registry',
Copy link
Member

Choose a reason for hiding this comment

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

we also need plone.api here. in plone 5, this is even a core dependency, although it isn't used in the core

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll remove plone.api so it can be better forwarded to Plone 5.0

@tisto
Copy link
Sponsor Member

tisto commented Sep 17, 2015

@petschki @thet plone.api is not allowed to use in core. We ship with it but we can't use it. Please amend the code to use the plone.app.registry lookups.

@jaroel we need to set up a jenkins check to make sure plone.api is not used in the core. Otherwise we will not be able to control this. This use case is more complex than grok since we ship with plone.api but we can't use it.

@thet
Copy link
Member

thet commented Sep 17, 2015

@tisto regarding plone.api: that change aims only for the 1.x branch, which will never be part of the core. However, I'd also prefer the plone.app.registry lookup, since this package is quite close to the core.

@tisto
Copy link
Sponsor Member

tisto commented Sep 17, 2015

@thet right. plone.app.widgets has been merged by @vangheem into other plone packages...ok, then I don't care. ;)

@thet
Copy link
Member

thet commented Sep 17, 2015

@petschki - the travis build errors can be fixed with that commit: 87e924c

@petschki
Copy link
Member Author

@thet cool ... tests are passing now. note: I've changed the default time interval back to 5 minutes

@thet
Copy link
Member

thet commented Sep 17, 2015

Merged via: 1dcfa48

@thet thet closed this Sep 17, 2015
@thet thet deleted the registry_settings branch September 17, 2015 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants