-
Notifications
You must be signed in to change notification settings - Fork 10
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
OZ-657: Make scanBasePackages
configurable
#37
Conversation
## --- EIP Client Configuration ---------------------------------------------------------------------------------------- | ||
# This is a comma-separated list of packages to scan for EIP classes (components, routes, etc.) | ||
# Default is org.openmrs.eip.* which will scan all packages under org.openmrs.eip. However, you can specify a more | ||
eip.app.scan.packages=org.openmrs.eip.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adds default as to not break existing projects(e.g eip-odoo-openmrs
). I believe this should configured from the respective route project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is org.openmrs.eip.*
optional because not all users need the core EIP entities? And are there mandatory entity classes in com.ozonehis.eip.*
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is org.openmrs.eip.* optional because not all users need the core EIP entities?
Yes, it's optional. For example demo service and EIP Keycloak Superset.
And are there mandatory entity classes in com.ozonehis.eip.*
Currently, all routes projects within Ozone uses com.ozonehis.eip.*
as the base package. Plus other common utils bean e.g ShutdownHandler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, LGTM. I have one minor suggestion for a documentation change below.
Co-authored-by: Ruhanga <41738040+Ruhanga@users.noreply.github.com>
@@ -12,7 +12,7 @@ | |||
import org.springframework.boot.autoconfigure.SpringBootApplication; | |||
|
|||
@Slf4j | |||
@SpringBootApplication(scanBasePackages = {"org.openmrs.eip, com.ozonehis.eip"}) | |||
@SpringBootApplication(scanBasePackages = "com.ozonehis.eip.*, ${eip.app.scan.packages}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we try this
@SpringBootApplication(scanBasePackages = {"com.ozonehis.eip.*", "${eip.app.scan.packages}"})
Issue: https://mekomsolutions.atlassian.net/browse/OZ-657
This PR makes
scanBasePackages
configurable.