-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
Import loader from YAFOWIL. #98
Conversation
Without doing this, various built-in widgets aren't registered, which results in mysterious KeyError messages when people try to configure LDAP in Plone. Fixes collective#97 and collective#92.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixing things with a magic import makes me suspicious in general, but finding a cleaner solution might not be worth the effort. So I'm OK with it for the moment.
@reinhardt FWIW, the "magic import" is what the YAFOWIL documentation tells you to do. My guess (I haven't checked) is that what happened that broke it is that someone set things up to check everything with Flake8, which said it wasn't used, so it got deleted. That might not have broken on some peoples' setups because they may be doing that import in some other module, so YAFOWIL ends up with everything registered properly because it's already pulled in elsewhere. I'm not a huge fan of the magic import either, but IMO that's really something that would need addressing in YAFOWIL rather than here. |
I agree. If @jensens has no further reservations I would merge it like this. |
the import of yafowil.loader actually registers all blueprints of yafowil and addon widgets to the yafowil factory. Maybe we change this in future and provide a dedicated function call to avoid confusion. Though, as this would be an API break, it probably will not happen very soon. |
someone should drop a release. @jensens? |
I can do that. I'll make it 1.7.2. |
... except that my release setup seems to be broken in some way. I'd be grateful if someone else could take care of it after all. |
Ah! Now it worked. Strange. But that should be all done now. |
I just installed |
same problem here: fresh install of pas.plugins.ldap (1.7.2) and i have the same error of @NicolasGoeddel |
Without doing this, various built-in widgets aren't registered, which
results in mysterious KeyError messages when people try to configure
LDAP in Plone.
Fixes #97 and #92.