results: 3 +1 1 -0 0 -1
Changes committed. see: http://jira.xwiki.org/jira/browse/XWIKI-4779 and: http://jira.xwiki.org/jira/browse/XWIKI-4741 I decided to go with the name $captchaservice because it is closest to the names of other velocity bindings which are defined with VelocityContextInitializer. I don't like the idea of binding to an arbitrary velocity variable but I don't think it improves the situation to use a confusing naming convention which I don't see implemented anywhere else. see: $l10n http://svn.xwiki.org/svnroot/xwiki/platform/core/trunk/xwiki-localization/src/main/java/org/xwiki/localization/internal/LocalizationManagerVelocityContextInitializer.java $officeimporter http://svn.xwiki.org/svnroot/xwiki/platform/core/trunk/xwiki-officeimporter/src/main/java/org/xwiki/officeimporter/internal/OfficeImporterVelocityContextInitializer.java $ooconfig http://svn.xwiki.org/svnroot/xwiki/platform/core/trunk/xwiki-officeimporter/src/main/java/org/xwiki/officeimporter/internal/openoffice/OpenOfficeConfigurationVelocityContextInitializer.java Caleb Thomas Mortagne wrote: > On Thu, Jan 21, 2010 at 23:25, Vincent Massol <[email protected]> wrote: >> On Jan 21, 2010, at 10:32 PM, Thomas Mortagne wrote: >> >>> On Thu, Jan 21, 2010 at 22:23, Caleb James DeLisle >>> <[email protected]> wrote: >>>> I am adding a new captcha component, it uses jcaptcha-2.0-alpha1 and >>>> supports >>>> image captchas and sound captchas (freetts jars must be added for sound >>>> captcha support) >>>> >>>> There is a captcha plugin in xwiki-core which makes the core depend on >>>> jcaptcha-all-1.0-RC3. >>>> jcaptcha-all-1.0-RC3 causes conflicts when getting classes from >>>> jcaptcha-2.0. >>>> The plugin in querstion: >>>> http://svn.xwiki.org/svnroot/xwiki/platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/plugin/captcha/ >>>> >>>> Here's my +1 for removing it entirely, breaking backward compatibility, >>>> lightening xwiki-core, >>>> and making the core no longer depend on jcaptcha. >>>> You may review a patch for removing the plugin here: >>>> http://jira.xwiki.org/jira/secure/attachment/16508/XWIKI-4779-removeJcaptchaPlugin.patch >>> -0 for removing captcha plugin >>> >>> I don't like breaking something when it's not absolutely necessary, >>> would it be too much work (maybe it's not even possible) to refactor >>> xwiki-core captcha plugin to use jcaptcha-2.0 ? >> I think there's some misunderstanding thomas. Caleb has developed a >> component-based captcha module. > > No there is not. This vote is about removing xwiki-core public API. > The fact that it's a plugin has nothing to do with that. > >> -1 to not remove the old captcha plugin ;) >> >> There's no point of having 2 systems that do the same thing. At worst the >> old plugin could be moved to retired projects in contrib but I don't even >> think we should do that since nobody is using the old catpcha plugin AFAIK. >> Plugins are meant to be removed. They're the old system that has been >> replaced by components. We should definitely not spend any time trying to >> create new plugins. > > It's not so simple, plugin or not that's xwiki-core module public API > not some external plugin and i don't like removing public API if not > necessary.. We don't use it by default in XE does not mean nobody is > using it, if you look at the mailing list it seems at least some > people are using it. > >> In addition it would be a pain and completely unnecessary to maintain 2 >> modules that do the same thing but differently. > > I just talked about API here, we could maybe use the new component > behind old plugin APIs. Also my vote is not blocker, i just asked a > question: the proposal is to remove a public API instead of deprecate > it without explaining why compared to the general rule. > >> Thanks >> -Vincent >> >> >>>> To provide velocity access to the captcha component for checking the >>>> results of the captcha, >>>> I would like to add a VelocityContextInitializer which will add a binding >>>> called $captchaservice >>>> which will provide 3 methods: >>>> >>>> /** >>>> * If this is false, the captcha system will still work. >>>> * It is for velocity scripts to determine whether captchas are needed. >>>> * >>>> * @return Is the captcha service enabled in the configuration. >>>> */ >>>> boolean isEnabled() >>>> >>>> >>>> /** @return a List of the names of all registered classes implementing >>>> {...@link org.xwiki.captcha.CaptchaVerifier}. */ >>>> public List<String> listCaptchaNames() >>>> >>>> /** >>>> * Get a {...@link org.xwiki.captcha.CaptchaVerifier} from the service. >>>> * >>>> * @param captchaName The name of the type of captcha, use {...@link >>>> #listCaptchaNames()} for a list >>>> * @return A captcha object which can be used to check a specific answer >>>> for a given challange >>>> * @throws Exception If the named captcha doesn't exist. >>>> */ >>>> public CaptchaVerifier getCaptchaVerifier(String captchaName) throws >>>> Exception >>> Don't you have something more precise than Exception ? It does not >>> seems right to me, it should be a specialized exception >>> (org.xwiki.captcha.InvalidNameException or something). Except for unit >>> tests a methods that throws Exception generally mean there is >>> something wrong in the design. >>> >>>> >>>> I realize throwing an Exception to velocity always results in a stack >>>> trace but I see it as the lesser evil to >>>> returning a non-functional CaptchaVerifier which always returns false. >>>> Also I see no use case where the name of >>>> the captcha is not given in hardcoded velocity so this Exception is only >>>> for the velocity developers. >>>> Here's my +1 for adding this as well. >>>> >>>> >>>> Caleb James DeLisle >> _______________________________________________ >> devs mailing list >> [email protected] >> http://lists.xwiki.org/mailman/listinfo/devs >> > > > _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
