-
Notifications
You must be signed in to change notification settings - Fork 261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove 'split packages' #7246
Comments
The referenced PR changes more than required (investigation work needed to resolve issues), so the renames there will be undone.
Java packages enforce more restrictions, but make it easier to check we're following our conventions, and over time allow us to clarify the expected visibility of packages Technically this is only required if we agree to adopt the module system, though split modules are generally not regarded as good practice - and we are extremely good in avoiding them! |
The split module analysis does highlight some problems that I had been avoiding fixing. However, the name changes for ffdc above break the naming conventions for the packages. The If ffdc is in both the api and server modules then I suggest moving the audit log definitions from the server module to the api module. This is consistent with many other modules and is a necessary change when we start generating message manuals. The naming of the packages around the component definitions in admin are confusing and could do with improving. This is worth doing anyway. I think the appropriate packages should be renamed rather than creating a deeper nested structure. No concerns about the changes to rest connectors. On the view services, the packages are typically .../server/spring rather than ../spring. |
I am happy to make the changes in main if that is helpful - or it is going to cause problems with the experiment? |
@mandy-chessell The experiment went a bit too far with renaming ;-) I need to untangle that. I wanted to understand the scope first.They were never meant to be the final proposal. Happy to go with names that make the most sense to you. The key issue we have is 'split packages' ie where the same package name is represented in multiple jars. There's actually very few, but it is completely disallowed for java modules. No warnings etc. it's just a no. (there are magic workarounds which manipulate the jars/classpath .. but best to keep those to third party libraries one can't control) Suggestions - one of
Then I can remove my class refactoring, & pull in your update & revalidate on my PR -- I'm keeping the module support another level apart for now until we've had a chance to discuss, plus not completed sorting the test cases yet. Most of the changes for modules are in the creating of the module-info.java & gradle dependencies (to a lesser extent) If there are changes, splitting them up as much as possible helps with rebase/merge/review. Obviously any change will hit a lot of files, but one rename per commit would be really useful :-) Or one batch of self contained moves (keeping things working). Currently the module support PR has > 800 file changes :-( I thought it would be pretty quick but it was a bit nasty in places. Alternatively if you spec out the renames I can do. |
Signed-off-by: Mandy Chessell <[email protected]>
Existing/related issue?
#2377
Please describe the new behavior that that will improve Egeria
With 'Java Modules' (in Java 9 and above), 'split packages' are not allowed.
This is where the same package name is found in two different modules (=jars).
See https://www.logicbig.com/tutorials/core-java-tutorial/modules/split-packages.html
Generally in Egeria we adhere to this principle, however a few anomalies were found when working on the referenced issue #2377
In the 'demonstration' PR #7245 the modified directories needed were:
These names can be modified, but the affected packages are:
org.odpi.openmetadata.adapters.connectors.restclients
org.odpi.openmetadata.adminservices
org.odpi.openmetadata.adminservices.configuration.registration
org.odpi.openmetadata.viewservices.dino.server
org.odpi.openmetadata.viewservices.rex.server
org.odpi.openmetadata.viewservices.tex.server
Alternatives
No response
Any Further Information?
No response
Would you be prepared to be assigned this issue to work on?
No response
The text was updated successfully, but these errors were encountered: