-
Notifications
You must be signed in to change notification settings - Fork 90
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
Solves #1166 #1179
Solves #1166 #1179
Conversation
@@ -49,6 +49,7 @@ E.g. for includeDependenciesScopes could be configured as: | |||
- `configProperties` (String) - Load any properties from a file. Example `${basedir}/src/main/resources/application.properties`. | |||
- `attachArtifacts` (boolean, default: false) - Attach the built OpenAPI schema as build artifact. | |||
- `skip` (boolean, default: false) - Skip execution of the plugin. | |||
- `encoding` (String) - Encoding of output OpenAPI files. |
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.
We can probably make the default UTF-8 to stay backward compatible ?
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.
@phillip-kruger, I thought the same thing, but previously it was using String#getBytes
, which internally calls Charset.defaultCharset
.
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.
Ok, but it should still be able to do this. Not sure if you can make these properties Optional ? If so , use Optional with empty setting to default
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.
The default defaultValue
in the @Parameter
is an empty string. That flows into the (!StringUtils.isBlank(encoding))
check and then uses the platform default encoding.
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.
Ok great !
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.
So then we can merge ?
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.
Yeah, lgtm!
/** | ||
* Output encoding for openapi document. | ||
*/ | ||
@Parameter(property = "encoding") |
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.
So here defaultValue = "UTF-8"
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 !
Added encoding as parameter to set output file encoding