-
Notifications
You must be signed in to change notification settings - Fork 280
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
codegen-java: Support not annotating constructor parameters #792
base: main
Are you sure you want to change the base?
Conversation
dc475d5
to
a06c301
Compare
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; makes sense!
Looking for at least one more thumb from @stackoverflow or @holzensp.
Example: `paramsAnnotation = "org.project.MyAnnotation"` + | ||
Fully qualified name of the annotation type to use for annotating constructor parameters with their name. + | ||
The specified type must have a `value` parameter of type `String` or the generated code may not compile. + | ||
This option cannot be combined with `noParamsAnnotation`. |
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.
Thanks for filling these in!
Default: (flag not set unless `--generate-spring-boot` is set) + | ||
Flag that indicates not to annotate constructor parameters with their name. + | ||
This option cannot be combined with `--params-annotation`. |
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.
Would be good to add a blurb explaining how to use this. Same blurb should be copied to the pkl-gradle docs
Default: (flag not set unless `--generate-spring-boot` is set) + | |
Flag that indicates not to annotate constructor parameters with their name. + | |
This option cannot be combined with `--params-annotation`. | |
Default: (flag not set unless `--generate-spring-boot` is set) + | |
Flag that indicates not to annotate constructor parameters with their name. + | |
This option cannot be combined with `--params-annotation`. | |
If `true`, requires that Java is compiled with the `-parameters` flag, or that `-generate-spring-boot` is set, and classes are loaded by Spring Boot. |
Today I discovered a valid reason not to generate My original attempt was to change
But perhaps this is nevertheless a better approach? I'd really like to support disabling both |
What is the reason to not generate |
I’d like to support |
Gotcha. The reason we went with I'd be happy to accept a PR for this, though. |
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.
Looks good!
a06c301
to
4f7912d
Compare
After doing more research, I've come to the conclusion that disabling annotations with |
4f7912d
to
58cd968
Compare
3fd0783
to
39d2195
Compare
Motivation: Spring Boot configuration classes neither require nor benefit from annotating constructor parameters with their name. The same is true for pkl-config-java configuration classes compiled with `-parameter`. Changes: - Change CLI parameter `--params-annotation` to accept a `none` value. This is recommended in https://clig.dev/#arguments-and-flags and is how the `-F` parameter of the `ssh` command works. - Change `paramsAnnotation` property in Gradle plugin, CliJavaCodeGeneratorOptions, and JavaCodegenOptions as follows: - Change meaning of `null` from "generate org.pkl.java.config.mapper.Named annotations" to "do not generate annotations". This is a breaking change (only) affecting users who explicitly set the property to `null` instead of omitting it. - Change property default from `null` to: `null` if `generateSpringBootConfig` is `true` and `org.pkl.java.config.mapper.Named` otherwise - add tests - update docs of this and other codegen options Result: Generated code does not contain unnecessary annotations.
39d2195
to
b091f21
Compare
Motivation:
Spring Boot configuration classes neither require nor benefit from annotating constructor parameters with their name. The same is true for pkl-config-java configuration classes compiled with
-parameter
.Changes:
noParamsAnnotation
parameter to CLI, Gradle plugin, CliJavaCodeGeneratorOptions, and JavaCodegenOptionsgenerateSpringBootConfig
is setparamsAnnotation
andnoParamsAnnotation
are setResult:
Generated code does not contain unnecessary annotations.