-
Notifications
You must be signed in to change notification settings - Fork 402
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
refactor: use equivalent, shorter Builder.Owns()
method
#881
Conversation
instead of longer `Builder.Watches()`.
@kevin85421 @Jeffwan as recent committers/owners of the repo. Very small PR. ~5min to review docstring for
|
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. I am not very familiar with controller-runtime, so cc @DmitriGekhtman @architkulkarni for more reviews.
- docstring mentioned by @davidxia
- For me,
Owns
has a better compatibility with the master branch of controller-runtime thanWatches
. See [Question] Is IsController compatible with the master branch? kubernetes-sigs/controller-runtime#2140 for more details.
|
This is valid. The Watches on Events looks a little different -- what's going on there? |
Not sure. I left that line as is. Looks like it's listening for all events and was added in #294. More details in that PR.
|
Test failures due to known flakiness. |
instead of longer
Builder.Watches()
.