-
Notifications
You must be signed in to change notification settings - Fork 218
feat: make it possible to also check annotations/labels when matching #1393
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
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.
What is not clear to me in this impl, how it should be used in a real life dependent resource, that should override the match
method?
Should we showcase that somehow?
@@ -169,26 +198,26 @@ instances are managed by JOSDK, an example of which can be seen below: | |||
```java | |||
|
|||
@ControllerConfiguration( | |||
labelSelector = SELECTOR, |
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 don't get this, we are both using the same formatter from maven, how was this not formatted? It should fail also on build if not properly formatted
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 don't know but I've seen weird things going on with format… it doesn't appear as deterministic as it should be (or rather, the determinism is not obvious to me) 😓
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
Kudos, SonarCloud Quality Gate passed! |
…#1393) Fixes #1392 Co-authored-by: csviri <[email protected]>
…#1393) Fixes #1392 Co-authored-by: csviri <[email protected]>
…#1393) Fixes #1392 Co-authored-by: csviri <[email protected]>
Fixes #1392