-
Notifications
You must be signed in to change notification settings - Fork 6
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
Implemented ServiceLoader in Entrypoint #4
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Alex Ellis (OpenFaaS Ltd) <[email protected]>
Signed-off-by: Kamesh Chauhan <[email protected]>
Thanks for contributing this @kameshchauhan . I've also worked on #3 . Could you do a review please? |
Thank you for your contributions. I think we already have this covered though. Can you take a look at yesterday's commits? |
@@ -0,0 +1 @@ | |||
com.openfaas.model.SampleAbstractHandler |
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.
I believe this should be on the test resources and not on main.
The PR adds tests for service loader. |
Given that this PR is trying to do something we have now, we should probably close it, unless there's something else we want from it in a new PR? |
@alexellis , Please see the diff. The PR adds a test for the service loader functionality. |
This can't be merged until the commits are cleaned up, currently there are around 15 which are rather messy but also include unnecessary merge commits. Starting a new branch may be easiest, or running git reset or git rebase. Plenty of info online for how to do this. The title of the PR would also need to be updated as per the commit i.e. "add a test for the service loader". |
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 see the comments
Description
Motivation and Context
Which issue(s) this PR fixes
Fixes #
How Has This Been Tested?
Types of changes
Impact to existing users
Checklist:
git commit -s