-
Notifications
You must be signed in to change notification settings - Fork 459
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
Migrated to Operator SDK 0.19 #31
Migrated to Operator SDK 0.19 #31
Conversation
@kevinearls, would you like to review the tests? |
@jpkrohling I'd like to but unfortunately probably won't have a chance until Monday |
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.
Sorry didn't get far will continue next week.
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.
Another partial review
|
c49062b
to
0ad549a
Compare
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.
LGTM - a few minor comments.
Would be good to change upgrade to the semver approach now used in jaeger operator, but that can be in a separate PR.
38ce5c9
to
4a88da9
Compare
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
4a88da9
to
b203820
Compare
Sorry, I force-pushed a squashed version of the PR when trying to figure out the CLA failure. I promise I didn't change anything other than the places where you commented :-) |
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
Items to be completed in a follow-up PR:
Tests for the specific reconciliation tasks (config maps, deployments, ...)tracking issue: Tests for reconcilers #35Publishing of the container imagesPublish container images #36Change theAlign operator version with the tooling around it #37version
to not have av
prefix, in line with what the kubebuilder tooling expects. We can add the prefix where we expect thev
prefix to existCloses #23
Signed-off-by: Juraci Paixão Kröhling juraci@kroehling.de