238: Remove recursive classloader check in loadSpi #239
Conversation
| @@ -23,5 +23,5 @@ | |||
| * provide additional functionality for MP Rest Clients. | |||
| * | |||
| */ | |||
| @org.osgi.annotation.versioning.Version("1.1") | |||
There was a problem hiding this comment.
any reason why a patch over minor?
There was a problem hiding this comment.
I was thinking about removing the setInstance method per issue #103 - which would force the package version to be 2.0, but then I realized that we hadn't actually come to a consensus on that. Since this change isn't changing any method signatures, it shouldn't require a minor update. I'm not an expert in semantic versioning, but I think in this case, a micro version update is correct.
I'd also like to separate the "spi" package into it's own artifact - that might require a version change too, but I'm not positive about that.
There was a problem hiding this comment.
Took a quick look at semVer and it probably does align with a patch version
|
@andymc12 do you know what might have been the reason for the previous approach? |
|
@michalszynkiewicz I'm not sure. The previous approach was copied almost 100% from MP Config (and then changed to use RestClientResolver instead of ConfigResolver). This change is staying in sync with the changes to MP Config (and other specs) - and it looks right to me. The older approach was probably only useful in environments like Liberty which uses OSGi and isolates implementation classes from the application classloaders. I know that Liberty doesn't need this old functionality, so it should be safe there - it might be worth checking with other app servers to see if it works there - I suspect it is fine for Wildfly/Thorntail/Quarkus/etc. and GlassFish. I think TomEE uses OSGi, so it might be an issue there... @cesarhernandezgt is this something you could try out? |
|
FYI: TomEE does not use OSGi! |
|
@struberg thanks for the clarification. Given that, I think we are good to merge this change. Thanks all! |
Resolves issue #238 - using the same pattern as MP Config.