a Pull request will be a great tool here!!! we could discuss on top of actual changes.
On Mon, Nov 16, 2015 at 12:28 PM, Christian Schneider <[email protected]> wrote: > In the scope of the OSGi enablement of artemis I am currently looking into > the split package org.apache.activemq.artemis.utils. > The immediate problem we have is that this package is also present in the > artemis-commons project. So one simple solution would be to just move it to > a package > like org.apache.activemq.artemis.server.config.utils. > > The real problem is a bit deeper though. > > XMLConfigurationUtil refers to > org.apache.activemq.artemis.core.config.impl.Validators > So a non impl package refers to an impl package which is already not ideal. > We could move it to common/util/xml, and create an interface for the Validator.. so the impl could leave elsewhere and passed as a paremeter: Something along this line? https://github.com/clebertsuconic/activemq-artemis/commit/09d95f6e29f3921a94bfe5a43942d51c321240cf ^^ that's just a suggestion.. and part of the discussion here. It doesn't even compile as of right now. (from a branch I called suggestion on my fork for artemis) > XMLConfiguationUtil is used in: > - org.apache.activemq.artemis.core.config.impl.FileConfigurationParser > - org.apache.activemq.artemis.jms.server.config.impl.FileJMSConfiguration > > So at least the impact of changes should be relatively small. > > Additionally the Validators class refers to some other impl packages which > is also not so good. > import Moving to util and creating the interface would fix that? > org.apache.activemq.artemis.core.server.cluster.impl.MessageLoadBalancingType; > import > org.apache.activemq.artemis.core.settings.impl.AddressFullMessagePolicy; > import org.apache.activemq.artemis.core.settings.impl.SlowConsumerPolicy; > > All these classes are enums. Strangely these are in impl packages. I would > have expected them to be in the repective API. > So I think it would be a good step to move these enums one level up to the > API packages. > Then Validators would only refer to other API packages which is already > quite a bit better. Moving this would break the API, and the integration with user's and applications. One obvious user will be the Wildfly.. We could of course fix the integration here but that wouldn't fix anyone else using the same API. We could always move, extend it and deprecate the old package. > > As Validators and XMLConfigurationUtil are very closely related and deal > with XML config I propose to put them in the same package > org.apache.activemq.artemis.core.server.config.xml. > > What do you think? > If you consider to do this change I can send a pull request. > > Christian > > Btw. I also found some other small odd thing: > org.apache.activemq.artemis.core.deployers.impl.FileConfigurationParser > seems to be misplaced. > It seems to rather belong into org.apache.activemq.artemis.core.config.impl > where also its test resides. It will break API on integration? you could move it and keep an extension deprecated?
