On Sun, Aug 21, 2016 at 2:44 PM Mark Struberg <[email protected]> wrote:
> Oh if it's not public then we should really make it! > > You are right with 'production code'. Of course it's in production. > But it's more like infrastructure glue code and not business related, > right? > Means your colleagues tinkering on UI and business should typically not > even get it as compile scoped dependency but really only as runtime dep. > John, romain, do you agree on this point? > Right, the problem is that the class isn't even public. So even if I declare a proper dependency on it, can't be used. I would say my view point is that its SPI, along with a few other classes (MapConfigSource, PropertiesConfigSource, PropertyFileConfigSource). Realistically though, the problem I was trying to solve was resolved by implementing "PropertyFileConfig" but trying to remember how that should work, I couldn't even find the doc page (eventually found it, its mentioned briefly on core, but really should be its own link on the landing page, which I changed). I could also see the class not being SPI. As long as things are configured properly, its not needed. That's assuming we have a dynamic way to register more property files. Right now, that's not possible (PropertyFileConfig is only read once, and only via ServiceLoader). Maybe thats the better discussion to have. > Just trying to make a few steps back to look at the problem from a > distance... > > LieGrue, > Strub > > > Am 21.08.2016 um 16:11 schrieb Romain Manni-Bucau <[email protected] > >: > > > > 2016-08-21 16:08 GMT+02:00 Mark Struberg <[email protected]>: > > > >> EnvironmentPropertyConfigSourceProvider is effectively an internal > class. > >> And thus I'd not move it to the API. Just look how much we would need to > >> add to the api package. > >> > >> I understand that you don't like to duplicate code. > >> > >> Usually things like ConfigSources are totally orthogonal to your > >> production code. So you need those only as runtime dependency. > > I get the idea but this is likely not what you wanted to say. Typically > > sources are production code (but not business code). > > > > > >> > >> In that case just make a tool module which has ds-core-impl as compile > >> time dependency and then you can simply re-use and even extend the > internal > >> class. > >> But that class is clearly NOT an API class imo. > > Question is: how stable do we consider -impl is? If the solution to that > > issue is 1. make the class (whichever it is) public 2. say to the user to > > import -impl in his pom, then we need to show that the API is stable and > > not just an internal we can break when we want. @Stable would be an > option > > but moving it to -api is another one. Don't care of the solution while we > > prevent the user to duplicate our -impl code. > > > > > >> LieGrue, > >> strub > >> > >> > >> > >> > >> > >>> On Sunday, 21 August 2016, 15:28, John D. Ament <[email protected] > > > >> wrote: > >>>> Hi, > >>> > >>> I have a use case for leveraging something > >>> like EnvironmentPropertyConfigSourceProvider to load multiple property > >>> files, outside of apache-deltaspike.properties. Right now its package > >>> local and in the impl JAR. I'd rather not duplicate the logic since > >> whats > >>> being done is exactly what is needed. I was wondering if anyone had > any > >>> concerns with making this an API class instead of an impl class? > >>> > >>> John > >> > >
