-
Notifications
You must be signed in to change notification settings - Fork 167
Sdk bump v0.0.7 => v0.7.0 #159
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
Conversation
|
/test e2e-operator |
|
currently troubleshooting adding the elasticsearch scheme for the operator to run |
|
/test e2e-operator |
|
/retest |
1 similar comment
|
/retest |
|
/hold |
|
looks like fluentd isn't correctly being deployed/tested as part of the ci and this is hidden due to running tests in parallel. tests will begin to start failing for |
c88f30d to
f73db61
Compare
jcantrill
left 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.
It LGTM. Only question i have is ability to mock the client for testing which would mean the CRUD methods on clusterloggingrequest would move to interface. If mocking is easier then unnecessary
|
@jcantrill I'm currently looking into how we can do mocking with our tests and can make that part of this PR. |
|
/refresh |
|
/retest |
|
Outstanding job, Eric!! Really good! I only stumbled upon some unexpected comments. Please run |
|
I believe I need to update this PR to point to |
af5b0b2 to
8c66f8d
Compare
|
/lgtm |
|
/hold cancel |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ewolinetz, jcantrill The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Sdk bump v0.0.7 => v0.7.0
Depends on openshift/elasticsearch-operator#122