-
Notifications
You must be signed in to change notification settings - Fork 2
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
Move plugins to own repo #4
Conversation
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.
@rehammuzzamil can we remove the Google specific plugins and only keeps the OpenSRP ones
Sure, but I believe there are still certain classes that would be duplicated for example some helper classes. |
@rehammuzzamil for helper classes we may be using that are not part of the plugins package feel free to change their visibility e.g. make methods |
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.
@rehammuzzamil we can add some documentation on the main README.md file with overview on this project since it's an open source project.
See these links for samples:
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.
Please include the changes in these commits :
customplugins/src/main/java/org/smartregister/fhir/gateway/plugins/PermissionAccessChecker.java
Outdated
Show resolved
Hide resolved
customplugins/src/main/java/org/smartregister/fhir/gateway/plugins/PermissionAccessChecker.java
Outdated
Show resolved
Hide resolved
customplugins/src/main/java/org/smartregister/fhir/gateway/plugins/PermissionAccessChecker.java
Outdated
Show resolved
Hide resolved
customplugins/src/main/java/org/smartregister/fhir/gateway/plugins/PermissionAccessChecker.java
Outdated
Show resolved
Hide resolved
Let's configure spotless to handle thisOn Oct 3, 2023, at 14:08, Martin Ndegwa ***@***.***> wrote:
@ndegwamartin commented on this pull request.
In customplugins/src/main/java/org/smartregister/fhir/gateway/plugins/PermissionAccessChecker.java:
import com.google.gson.JsonObject;
import java.util.*;
import java.util.stream.Collectors;
import javax.inject.Named;
+import org.apache.commons.lang3.StringUtils;
import org.hl7.fhir.r4.model.*;
Ditto
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
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.
Actually on further review I've noticed there's a lot of important changes that are in the PR here opensrp/fhir-gateway#45 and my not be part of this PR e.g. the changes in the code block here https://github.com/opensrp/fhir-gateway/pull/45/files#diff-97e5f04f911ea8b59eb9a66205ba40363d78acb788a1eb396e3e827b4fb94d9aR353-R359
Please review the PR again and incorporate all the (relevant) changes. Pick them manually, cherry picking with Git will not work.
Noted, let me review it. Thanks |
@rehammuzzamil for now lets also remove all the licenses since the repo/codebase is private cc @pld |
Done |
@rehammuzzamil not sure you pushed your changes?? I still see this commit (among others) still missing. Perhaps we can pair so that we are aligned. |
@ndegwamartin Thanks! |
plugins/src/main/java/org/smartregister/fhir/gateway/plugins/PermissionAccessChecker.java
Outdated
Show resolved
Hide resolved
plugins/src/main/java/org/smartregister/fhir/gateway/plugins/PermissionAccessChecker.java
Outdated
Show resolved
Hide resolved
Artifact release version update to `opensrp-gateway-plugin`
Resolves opensrp/fhircore#2213 and resolves #5