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.

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

matchers ignore list #1880

Merged
merged 17 commits into from
May 4, 2023
Merged

matchers ignore list #1880

merged 17 commits into from
May 4, 2023

Conversation

csviri
Copy link
Collaborator

@csviri csviri commented May 2, 2023

No description provided.

@csviri csviri self-assigned this May 2, 2023
@csviri csviri requested a review from metacosm May 2, 2023 15:19
@csviri
Copy link
Collaborator Author

csviri commented May 2, 2023

@metacosm in a separate PR will change the behavior, in a way that Kubernetes Service will be a special case, but the default behavior will use equality matcher (if no objections).

@csviri csviri marked this pull request as ready for review May 3, 2023 07:25
@metacosm metacosm force-pushed the matchers-ignore-list branch 2 times, most recently from f431b34 to a96e6ad Compare May 3, 2023 10:35
public static <R extends HasMetadata> Result<R> match(R desired, R actualResource,
boolean considerMetadata) {
return match(desired, actualResource, considerMetadata, false);
return match(desired, actualResource, false, false, false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You changed the implementation but didn't change the javadoc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but that did not change the behavior I believe

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One parameter got changed from true to false. If changing this doesn't change the behavior, then what's the point of the parameter? 😄
More seriously, though, before this commit the metadata was considered, now it's not anymore so the javadoc is not correct anymore with respect to the implementation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm checking this, but actually cannot see, where. So it is not considered on the main now. Maybe we can take a look offline.

@sonarcloud
Copy link

sonarcloud bot commented May 4, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

85.6% 85.6% Coverage
0.0% 0.0% Duplication

@csviri csviri requested a review from metacosm May 4, 2023 12:12
[skip ci]
@csviri csviri merged commit e586e78 into next May 4, 2023
@csviri csviri deleted the matchers-ignore-list branch May 4, 2023 13:05
csviri added a commit that referenced this pull request May 4, 2023
Co-authored-by: Chris Laprun <[email protected]>
csviri added a commit that referenced this pull request May 24, 2023
Co-authored-by: Chris Laprun <[email protected]>
csviri added a commit that referenced this pull request May 30, 2023
Co-authored-by: Chris Laprun <[email protected]>
� Conflicts:
�	operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java
�	operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java
csviri added a commit that referenced this pull request May 31, 2023
Co-authored-by: Chris Laprun <[email protected]>
� Conflicts:
�	operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java
�	operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java
csviri added a commit that referenced this pull request Jun 8, 2023
Co-authored-by: Chris Laprun <[email protected]>
� Conflicts:
�	operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java
�	operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java
csviri added a commit that referenced this pull request Jun 16, 2023
Co-authored-by: Chris Laprun <[email protected]>
� Conflicts:
�	operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java
�	operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java
csviri added a commit that referenced this pull request Jun 20, 2023
Co-authored-by: Chris Laprun <[email protected]>
� Conflicts:
�	operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java
�	operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java
csviri added a commit that referenced this pull request Jun 21, 2023
Co-authored-by: Chris Laprun <[email protected]>
� Conflicts:
�	operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java
�	operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java
metacosm pushed a commit that referenced this pull request Jun 23, 2023
Co-authored-by: Chris Laprun <[email protected]>
� Conflicts:
�	operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java
�	operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GenericKubernetesResourceMatcher does not detect removals within StatefulSet.spec
2 participants