ok found some time to review this email at last. Sorry for the delay. On Oct 15, 2008, at 3:47 AM, Sergiu Dumitriu wrote:
> Hi devs, > > The new localization component is committed. I think it is working > pretty well, so it's time to decide what to do next (estimate > deadlines > and responsibles after each item): > > 0. Outside review - me and Marta refactored it about 5 times > already, a > third opinion would be nice. > ASAP, let's say by the end of the week - volunteers; this is a good > exercise, as it shows how to write and use components * I see you've used non threadsafe components for bundles. I don't think we should do that. * I'd prefer to use getMessage() instead of get() which is too cryptic for a LocalizationManager class IMO. * The Bundle implementations are actually BundleManager I think, each managing a different type of resources (ResourceManager which might be a better name) * I think there's an important bug. The LocalizationManager is supposed to be threadsafe but the use() method (I'm still unsure what it does and how it's used) doesn't look threadsafe * I'm surprised there are not tests. We must really write the tests either before the code or at the very least at the same time as otherwise they're loosing half of their power (which is to improve design) * WikiInformation doesn't seem to belong to this module. * I'm really missing tests to understand how the localization module is used. * Is it possible for an application to specify a single resource document and not use the other resource documents located in other applications in the wiki? I think either a namespace at the level of LocalizationManager (for example: getResourceCollection(String namespace)) or at the level of getMessage(String name, String namespace) (I prefer at the level of LocalizationManager) might be better. This also means introducing the concept of ResourceCollection as we have in the configuration module for configurations (ConfigurationCollection). * Is there a design document listing all the use cases we want to have somewhere? > 1. Add it in the build cycle (include it in the core pom as a > submodule) > ASAP - me After we agree on 0, sure. > 2. Deprecate the XWikiMessageTool class and methods. Should be > completely phased out by 1.9 according to our deprecation strategy. > ASAP - me +1 but it's possible we'll need the new velocity bridge to make the adaptation to the new component. I'd really like that we don't model the new component against the old message tool (like get() methods for ex). > 3. Use it in the xwiki-core, replacing the messagetool > Full replacement until 1.7M2 - me and other volunteers +0, I think this is not a priority for 1.7 (it's not even defined in our roadmap). I'd much much prefer you finish the Blog application overhaul which is planned and people are waiting for it. And finish the velocity module refactoring for uberspectors + ability to support macro reloading (not sure what's the status on that one). > 4. Propose a standard for writing keys > Next week - me + discussion on devs ok > 5. Start properly translating applications in their own documents > Until 1.7 final - everybody, especially application owners Shouldn't we start extracting applications from XE into their own modules first? > 6. Split ApplicationResources into smaller files, moving everything to > its proper place: .properties files in plugins, wiki documents for > applications, CoreResources.properties (the clean version of > ApplicationResources). ok > Until 1.7 final - everybody > 7. Improve the localization component: > 7a. Replace hashmaps with caches > Until 1.7M2 - me or volunteers > 7b. Write tests > Until 1.7M2 - me or volunteers Tests are a priority for validating the design. > A note for items 2 and 3: The localization component interface is > compatible with the message tool, as it provides the get(string) and > get(string, list) methods. However, old code casts the retrieved > object > to XWikiMessageTool, so replacing the "msg" context variable is not > advisable yet. And I really don't like using get(). Again I'm 100% sure we shouldn't consider the old message tool to model our new components. > I used 'l10n' as the variable name for the new tool, as > it reflects better what it does. And, as Vincent suggested, this is a > workaround until the velocity bridge is in place, when we shouldn't > use > direct context variables, but request access to services through this > bridge. Yes and at that time we should be able to map the old msg.get() to the new code. > From me, +1 to all of these. > WDYT? Any volunteers where needed? Thanks -Vincent _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
