Thanks for adding a link to the config stuff in a more visible place! Yes, I also think we should have those classes public (but in the impl module).
Regarding the PropertyFileConfig: I think I documented this, but better mention it again: You can register it in 2 ways 1.) Simply implement the interface -> in that case it gets picked up via ProcessAnnotatedType and will _only_ be available after the container did finish booting! That means that you cannot use values from those ConfigSources during CDI boot, e.g. for @Exclude 2.) To work around the timing issue in 1 we later also made it pick up via java.util.ServiceLoader. In that case it will get picked up like a regular ConfigSource and is also available during container bootstrap. Just don't fortet to add a @Exclude annotation to prevent the PropertyFileConfig from being picked up twice. Happy to get any feedback about how to improve the docs if something is missing. LieGrue, strub > On Sunday, 21 August 2016, 20:57, John D. Ament <[email protected]> wrote: > > 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 >> >> >> >> >
