[GitHub] [tomee] rmannibucau commented on pull request #713: TOMEE-2324 - Remove Corba ORB APIs from Codebase

[GitBox <gi...@apache.org>]

rmannibucau commented on pull request #713:
URL: https://github.com/apache/tomee/pull/713#issuecomment-734714050


   Maybe two notes checking this PR:
   
   1. generating the bytecode at build time can be an issue (if you check the TODO they will never happen with such an option). It can be trivial to keep the class as a .java, compile it with source=8 or create an org.apache.tomee:corba-integration module released independently (like javaee-api module) and depend on it.
   2. current impl moved to systematic reflection which will be slower and a bottleneck (synchronization) if still used
   
   Due to both points, I'd say that either this can be dropped as a feature or it needs some more love. A well working option would be to.
   
   1. create a SPI abstracting this API
   2. ensure it is in SystemInstance (keep in mind you can put in system properties or system.properties the spi-qualified-class=implementation to load it)
   3. use the SPI through SystemInstance services instead of the reflection
   
   Then the implementation of 1 can be either in a module in the project or not but it can stay in plain java if corba support is still desired.
   
   Indeed just my 2 cts but went that path and it only works for droppable code so thought I would share that feedback ;).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org