Hi, >> If done correctly, major new development can be done in the trunk >> without risk. > > Sure, if it only concerns a single location or limited set of locations. If > it touches multiple places, this is not that easy.
USE_DATA_STORE is used in multiple places, I don't think it's a big problem. And at some point the setting can be removed (I think it could be removed by now). > I basically like the idea but have my reservations with respect to system > property use: It makes it difficult in embedded situations and even almost > makes it impossible to use in app server or servlet container environments. This system property is only enabled for testing. If disabled by default, it can be enabled to test an unreleased feature. If enabled by default, it can be used as a 'kill switch' to test if this functionality is responsible for a problem. I don't see a use case where you want to start multiple repositories with different settings. > For example: you want to have two repositories in the same Java VM, one with > and one without data store.... I don't see a use case for that currently. The system property is not something that people need to or should use (unless they want to explicitly test). > If is really just for testing, a "public" field is bad, because this quickly > becomes (de-facto) API. And a branch may still be better suited. It is a final field, and initialized only once. The system property is not final, but if clearly documented I don't think there is a risk people use it in production. This flag is used instead of branching, and given the disadvantages of branching I think it is worth thinking about it. > Probably better -- also for visibility -- would be to have a functionality > to provide this kind of setup as part of the Repository descriptors. In that case there is a bigger risk that people use it in production I think. Also, it couldn't be used in all components: only in those that have access to the repository. > Maybe the use case is slightly wrong in that the InternalValue > should just use a "store", which in turn either uses the new data store code > or the old code and has to be setup somehow) It is doing that: it is eighter using BLOBValue (the old mechanism) or BLOBIn* (the new mechanism). InternalValue acts as the factory. Of course you could add another class that acts as the factory, but it wouldn't really simplify things. Regards, Thomas
