Skip to content

Commit

Permalink
let p.a.event be conditionnal
Browse files Browse the repository at this point in the history
  • Loading branch information
@kiorky
kiorky committed Mar 18, 2013
1 parent fdeca91 commit 3856916
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 7 deletions.
2 changes: 2 additions & 0 deletions .gitignore
View file
@@ -0,0 +1,2 @@
*.pyc
*.egg-info
3 changes: 2 additions & 1 deletion CHANGES.txt
View file
Expand Up @@ -4,7 +4,8 @@ Changelog
0.9.16 (unreleased)
-------------------

- Nothing changed yet.
- fix broken imports, have plone.app.events conditionnaly loaded
[kiorky]


0.9.15 (2012-07-02)
Expand Down
18 changes: 12 additions & 6 deletions Products/PloneTestCase/setup.py
View file
Expand Up @@ -15,6 +15,12 @@
from version import PLONE50


try:
import plone.app.event
HAVE_PLONE_APP_EVENT = True
except:
HAVE_PLONE_APP_EVENT = False

def install_products():
ZopeTestCase.installProduct('CMFCore', quiet=1)
ZopeTestCase.installProduct('CMFDefault', quiet=1)
Expand Down Expand Up @@ -147,13 +153,12 @@ def install_products_50():
if PLONE30:
default_base_profile = 'Products.CMFPlone:plone'

if (PLONE40 or PLONE41) and not PLONE50:
default_extension_profiles = ('plonetheme.sunburst:default',
'plone.app.event:default',)

if PLONE50:
default_extension_profiles = ('Products.ATContentTypes:default', )
default_extension_profiles += ('Products.ATContentTypes:default', )

if HAVE_PLONE_APP_EVENT and (PLONE40 or PLONE42) and not PLONE50:
default_extension_profiles += ('plonetheme.sunburst:default',
'plone.app.event:default',)

def setupPloneSite(id=portal_name,
policy=default_policy,
Expand Down Expand Up @@ -327,7 +332,8 @@ def _setupPackages(self):
ZopeTestCase.installPackage('plone.app.blob', quiet=1)
if PLONE42:
ZopeTestCase.installPackage('plone.app.collection', quiet=1)
ZopeTestCase.installPackage('plone.app.event', quiet=1)
if HAVE_PLONE_APP_EVENT:
ZopeTestCase.installPackage('plone.app.event', quiet=1)

def _setupHomeFolder(self):
'''Creates the default user's member folder.'''
Expand Down

4 comments on commit 3856916

@davisagli
Copy link
Member

Choose a reason for hiding this comment

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

IMHO, since plone.app.event is optional, we should not have PloneTestCase set it up at all, but leave that to the test setup of packages that depend on it. Or am I missing something?

@kiorky
Copy link
Member Author

@kiorky kiorky commented on 3856916 Apr 24, 2013

Choose a reason for hiding this comment

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

That's why in the first place i made this test, that's not me that added it to plonetestcase but i got failures once the setup was there on my own unrelated tests...

@thet
Copy link
Member

@thet thet commented on 3856916 Apr 30, 2013

Choose a reason for hiding this comment

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

@kiorky @davisagli i changed to install plone.app.event only for PLONE44 and up (f0d3ce5), altough @kiorky conditional checking was fine too... but this matches, how the things are done in this module

@davisagli CMFPlone needs a event type for it's CalendarTool tests. to avoid an hard dependency on p.a.e. in P.CMFPlone, I think it's better to set p.a.e in P.PTC (which will go away anytime, IIRC).

@kiorky
Copy link
Member Author

@kiorky kiorky commented on 3856916 Apr 30, 2013

Choose a reason for hiding this comment

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

Problem is that others addons like plone.app.users used on plone43 also will need plone.app.event, here we get a problem...

Please sign in to comment.