Introduce specific annotations for the generators #4348
Conversation
andreaTP
force-pushed
the
validation-annotations
branch
from
8800901
to
4ac08ae
Compare
andreaTP
force-pushed
the
validation-annotations
branch
from
4ac08ae
to
e2cfb20
Compare
| public @interface Max { | ||
| double value(); | ||
| } |
There was a problem hiding this comment.
For the follow-ups, it might be interesting to add Javadoc to each annotation properly documenting its usage, and maybe linking the Kubernetes spec equivalent.
There was a problem hiding this comment.
I'm failing to find a good "linkable" source with the specific field description, e.g.:
https://kubernetes.io/docs/reference/kubernetes-api/extend-resources/custom-resource-definition-v1/
Do you have any idea?
There was a problem hiding this comment.
No, I only found the go types sources when I was evaluating the field precision of the Max and Min configurations.
There was a problem hiding this comment.
added a generic reference to: jsonschemaprops K8s API reference ... the best I have found in the wild ...
| @@ -78,6 +79,11 @@ | |||
| public static final String ANNOTATION_JSON_IGNORE = "com.fasterxml.jackson.annotation.JsonIgnore"; | |||
| public static final String ANNOTATION_JSON_ANY_GETTER = "com.fasterxml.jackson.annotation.JsonAnyGetter"; | |||
| public static final String ANNOTATION_JSON_ANY_SETTER = "com.fasterxml.jackson.annotation.JsonAnySetter"; | |||
| public static final String ANNOTATION_MIN = "io.fabric8.generator.annotation.Min"; | |||
There was a problem hiding this comment.
All annotations should be put in the same package, imo.
There was a problem hiding this comment.
At this point we have annotations in 2 places:
io.fabric8.generator.annotation-> generic modifiers to represent CRD properties in Java codeio.fabric8.crd.generator.annotation-> specific annotations to tweak specifically thecrd-generator
Do you see any other invariant @metacosm ?
There was a problem hiding this comment.
As soon as the annotations are used in different contexts, it would be better for users to help discoverability, in particular, for the annotations to be in the same location even though they initially were developed for different purposes… Obviously, changing annotations location at this point wouldn't be backwards compatible so maybe just a matter of documentation? It might make sense to explain why they live in different packages so that users (and future maintainers, down the line 😸) don't think it's an arbitrary decision.
There was a problem hiding this comment.
Added a brief JavaDoc on each annotation, let me know if you want other/more changes for this 🙂
|
SonarCloud Quality Gate failed. |
|
@manusa ready to merge? |
|
Releasing 6.1.1 ATM |
|
Thanks for the heads up @manusa ! |
There was a problem hiding this comment.
Hi @andreaTP (CC @manusa) - I was about to deal with #3870 by rebasing on this PR branch and implement the generation of validation annotations in the java-generator but I am thinking that it wouldn't solve the original issue, since no runtime ser/de validation would occur (see my comment to the initial PR you provided).
WDYT? Should we move on with a separate java-generator issue?
I'm merging this one, so please, do so. |
* Introduce specific annotations for the generators * add javadocs
Description
Fix #4224
Fix #2959
Follow up from #4298 , introducing annotations for the fields that we want to support/map.
Type of change
test, version modification, documentation, etc.)
Checklist