Thank you Jon! On Fri, Jan 4, 2019 at 5:35 PM Jonathan Gallimore < jonathan.gallim...@gmail.com> wrote:
> 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 > > > > > >>> > > > > > > > > > > > > > > >
