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
[crd-generator] Fix inconsistent additionalPrinterColumns jsonPath #4614
Conversation
@@ -40,41 +42,42 @@ public AddAdditionPrinterColumnDecorator(String name, String version, String typ | |||
|
|||
@Override | |||
public void andThenVisit(CustomResourceDefinitionVersionFluent<?> spec) { | |||
Predicate<CustomResourceColumnDefinitionBuilder> matchingColumn = col -> col.getName() != null && col.getName().equals(columnName); | |||
Predicate<CustomResourceColumnDefinitionBuilder> matchingColumn = col -> col.getName() != null | |||
&& col.getName().equals(columnName) && col.getJsonPath() != null && col.getJsonPath().equals(path); |
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.
Just highlighting the "real change" in this PR.
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.
Perhaps we should issue a warning when we encounter such a situation?
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 checked the behavior, and it looks consistent to me without the need for a warning.
Using the "Joke" CRD used in the tests and created a corresponding "Joke" CR:
apiVersion: "samples.javaoperatorsdk.io/v1alpha1"
kind: JokeRequest
metadata:
name: jr-any
spec:
category: Any
status:
category: Pun
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 enough about the details so LGTM :)
rebased and now this diff looks great 🙂 cc. @manusa |
SonarCloud Quality Gate failed. |
Description
This is a proposal to fix #4610
After this fix,
additionalPrinterColumns
with different paths will be emitted with colliding names. I'm not sure if this has implications.Type of change
test, version modification, documentation, etc.)
Checklist