Thomas Mortagne wrote:
On Fri, Nov 21, 2008 at 7:28 AM, Sergiu Dumitriu <[email protected]> wrote:
tmortagne (SVN) wrote:
Author: tmortagne Date: 2008-11-19 15:57:38 +0100 (Wed, 19 Nov 2008) New Revision: 14298 Log: XWIKI-2825: Create a generic script macro A quick review of the xwiki-script module:
- DefaultScriptContextManager and ScriptExecutionContextInitializer should be in the internal package. - ScriptExecutionContextInitializer is not declared in components.xml, thus it is not used. - getScriptContext calls the initializers on each call. Is that correct? Shouldn't the context be initialized only once?
It in initialized only once (there is an if if (scriptContext.getAttribute("util") == null) in it). I was in a hurry for the 1.7M3 release so I just follow the way it's done for Velocity.
I can't see it, perhaps you forgot to commit it?
- scriptContextInitializerList (which also contains a typo in its name) could be completely injected by Plexus, thus there's no need for Initializable and Composable on this class. Just declare a dependency on ScriptContextInitializer components.
How do you do that ? The context initializers are retrieved the same way in DefaultExecutionContextInitializerManager so I assumed it was not possible to inject a list of dependencies in component.xml.
Just declare it like a normal dependency. Plexus can see that it is a list and populate with all the implementations. I did this in the localization component.
- The javadocs on the interfaces could be improved... They are rather short right now.
-- Sergiu Dumitriu http://purl.org/net/sergiu/