-
Notifications
You must be signed in to change notification settings - Fork 258
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
ref/#2540-Move Search Logic from Search.execute() to FhirEngine.search() #2541
Conversation
…om-Search.execute()-to-FhirEngine.search()
the code is duplicated not moved. |
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.
the code is duplicated and not moved. also, i actually feel it might be better to move this method to the search package instead of the other way around.
@itstanany will you be able to open another pr to move the api declaration of the search method to the actual search package? thanks for your contribution! i will close this pr for now. |
@jingtang10 , the api declaration is already in search package, so if you see it is better to be in search package, there is no more actions to do. just leave it as it is. Thanks for your consideration |
the search and count apis are defined in the @aditya-07 @MJ1998 @santosh-pingle fyi and feedback welcome |
I noticed that My question is: Should these functions stay as part of the public API for the I'm a bit unsure of the best approach here. What are your thoughts? |
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Fixes #2540
Description
Clear and concise code change description.
I moved the search logic implementation from Search.execute to FhirEngine.search
For better separation of concerns and single responsibility principles
Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?
Type
Choose one: Code health
Screenshots (if applicable)
Checklist
./gradlew spotlessApply
and./gradlew spotlessCheck
to check my code follows the style guide of this project../gradlew check
and./gradlew connectedCheck
to test my changes locally.