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/
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
