Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove extra Kind() funcs for cr types #1837

Merged
merged 1 commit into from
Oct 3, 2019
Merged

Remove extra Kind() funcs for cr types #1837

merged 1 commit into from
Oct 3, 2019

Conversation

racevedoo
Copy link
Contributor

@racevedoo racevedoo commented Oct 1, 2019 •
edited

This PR removes a Kind() method used to workaround kubernetes-sigs/controller-runtime#406. It is no longer needed.

Fixes #1831

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

This was referenced Oct 1, 2019
Closed
Open
@anyasabo
Copy link
Contributor

anyasabo commented Oct 1, 2019

jenkins test this please

@racevedoo racevedoo force-pushed the remove-kind-funcs branch 2 times, most recently from b41489a to 844c25e Compare October 1, 2019 19:49
@anyasabo
Copy link
Contributor

anyasabo commented Oct 1, 2019

LGTM, if you can fix up the merge issues from a PR that snuck in before this one we should be good

@racevedoo
Copy link
Contributor Author

conflicts fixed

@anyasabo
Copy link
Contributor

anyasabo commented Oct 1, 2019

jenkins test this please

@anyasabo
Copy link
Contributor

anyasabo commented Oct 2, 2019

One last reference I just noticed: there is also a TODO in pkg/controller/common/keystore/user_secret.go that we should either address or remove. Was added originally in #1279

@racevedoo
Copy link
Contributor Author

@anyasabo removed the TODO. All the places where Finalizer was called already use .Kind directly now

@anyasabo
Copy link
Contributor

anyasabo commented Oct 2, 2019

jenkins test this please

@anyasabo
Copy link
Contributor

anyasabo commented Oct 2, 2019

Removing it sounds good to me, I am not sure that changing it back to consuming a runtime.Object would be a big usability win

@racevedoo
Copy link
Contributor Author

I guess there's no need for it to know about the whole runtime.Object for now

@anyasabo anyasabo merged commit 4976cb6 into elastic:master Oct 3, 2019
@anyasabo
Copy link
Contributor

anyasabo commented Oct 3, 2019

thanks @racevedoo !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thbkrkr thbkrkr thbkrkr approved these changes

@anyasabo anyasabo anyasabo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove Kind() funcs
4 participants
@racevedoo @elasticmachine @anyasabo @thbkrkr