Thanks for the explanation, I appreciate it. As long as there's no objections I'll merge the PR in.
Jon On Fri, Jan 4, 2019 at 6:28 AM Gurkan Erdogdu <cgerdo...@gmail.com> wrote: > Thanks Jon. > > I don't have any aim to replace service-jar.xml approach. We will just add > another YAML based configuration support. Therefore, all tomee.xml, > resources.xml etc will be stay in there. YAML is just an additional > feature. > > Introducing new module , tomee-config, allow us to centralise all > configuration into one place and all other modules can use it via > dependency. In the future, we can remove all configuration codes from > openejb-core, etc. into this module. There are lots of system properties > using in different modules, therefore each such module will add their > properties into TomEESystemConfig class and then use it. > > Currently, openejb-core is a one large module. We may also think to divide > this module into smaller modules which are having special purpose. > > Hope it clears your concerns. > > Regards. > Gurkan > > On Thu, Jan 3, 2019 at 11:30 PM Jonathan Gallimore < > jonathan.gallim...@gmail.com> wrote: > > > I commented on your PR - thanks for sending that over. I think it would > be > > worthwhile structuring the class with the constants in such a way that > the > > javadoc could end up on the website via David's site generation code. > That > > would be extremely cool and I'm sure a very useful piece of > documentation. > > > > While I appreciate your service-jar.xml visibility comment, I suspect > that > > is because of something missing in its documentation, as opposed to an > > issue with the design choice. I don't necessarily object to something > that > > can parse the config in YAML (I believe there is something that handles > > JSON already), providing the existing config mechanism in XML, with the > > defaults in service-jar.xml continued to work, and that I wouldn't be > > forced to migrate from XML to YAML. Many users make use of tomee.xml and > > resources.xml in a huge number of applications. Forcing a migration on > them > > to YAML "just because" doesn't make sense to me. The concept of the > > service-jar.xml is a very core design concept in TomEE and I would not be > > favour of changing it unless there was a really good rationale and a > better > > design proposed and discussed. > > > > I agree that system properties can lead to "magic" parameters that aren't > > well documented, and I'm definitely in favour of improving the > > documentation of those properties. The ability to override config with > > system properties, however, is useful and also widely used so we'll need > to > > keep it. > > > > Jon > > > > On Wed, Jan 2, 2019 at 9:53 AM Gurkan Erdogdu <cgerdo...@gmail.com> > wrote: > > > > > For me, using services-jar.xml approach is not so visible to users. All > > > defaults are configured in this file and packaged within JAR file. > Users > > > are not able to read the parameter explanation from services-jar.xml > > files. > > > Also, services.-jar.xml is somebit different from using the system > > > properties to turn-on/off something. I am also thinking to introduce > YAML > > > based configuration. > > > > > > But first step is to centralise all of these system parameters into two > > > classes. Maybe, we will see that some of them are unnecessary etc. > > > > > > On Wed, Jan 2, 2019 at 12:44 PM Bruno Baptista <bruno...@gmail.com> > > wrote: > > > > > > > Yes, there is. > > > > > > > > This is also the most basic MP spec and nothing prevents us from > using > > > > it everywhere. > > > > > > > > There might be Jakarta EE restrictions in how to handle > configurations > > > > that need to be assessed. > > > > > > > > Overall, I think that if we are going to mess with configs, we should > > > > use state of the art. > > > > > > > > Cheers > > > > > > > > Bruno Baptista > > > > https://twitter.com/brunobat_ > > > > > > > > > > > > On 02/01/19 09:35, Jean-Louis Monteiro wrote: > > > > > I think with microprofile-config we may have a chicken and the egg > > > > problem, > > > > > isn't it? > > > > > -- > > > > > Jean-Louis Monteiro > > > > > http://twitter.com/jlouismonteiro > > > > > http://www.tomitribe.com > > > > > > > > > > > > > > > On Wed, Jan 2, 2019 at 10:30 AM Bruno Baptista <bruno...@gmail.com > > > > > > wrote: > > > > > > > > > >> Hi Gurkan, > > > > >> > > > > >> I agree we have a problem with the documentation of the different > > > > >> properties and that we need to improve it. > > > > >> > > > > >> Doing the inventory and using the proposed syntax looks ok to me > > but I > > > > >> also think we should go even further. > > > > >> > > > > >> How about to migrate all the properties to use > microprofile-config? > > > > >> > > > > >> Cheers. > > > > >> > > > > >> Bruno Baptista > > > > >> https://twitter.com/brunobat_ > > > > >> > > > > >> > > > > >> On 02/01/19 07:20, Gurkan Erdogdu wrote: > > > > >>> Hello > > > > >>> There are lots of known and unknown system properties in the > > current > > > > code > > > > >>> base. I would like to introduce TomEESystemProperties and > > > > >>> OpenEJBSystemProperties classes to hold these system property > > > constants > > > > >> and > > > > >>> provide clear comment what it does. For example: > > > > >>> > > > > >>> class TomEESystemProperties{ > > > > >>> public static final String TOMEE_FORCE_RELOADABLE = > > > > >>> "tomee.force-reloadable"; > > > > >>> .... > > > > >>> } > > > > >>> > > > > >>> class OpenEJBSystemProperties{ > > > > >>> public static final String OPENEJB_CROSSCONTEXT_PROPERTY = > > > > >>> "openejb.crosscontext"; > > > > >>> .... > > > > >>> } > > > > >>> > > > > >>> WDYT? > > > > >>> Regards. > > > > >>> Gurkan > > > > >>> > > > > > > > > > >