Skip to content

Allow maven-codegen-plugin to set additionalProperties even if cliOptions does not define them #1970

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

Merged
merged 5 commits into from
Feb 16, 2016

Conversation

evigeant
Copy link
Contributor

The maven-codegen-plugin does not allow setting additionalProperties when they are not defined in cliOptions.

Several generators do not set the cliOptions properly, for example: html and dynamic-html. And in this case, the maven plugin cannot customize the behavior. The codegen-cli does not enforce this, so it can customize the behavior.

@fehguy
Copy link
Contributor

fehguy commented Jan 25, 2016

This solution seems backwards. How about instead we fix the offending language files and throw a warning when trying to set an option which has not been configured?

@evigeant
Copy link
Contributor Author

Ok, in this PR, I will make the change so that a Warning is emitted when an
undefined option is set, but I will not prevent it as this is limiting for
all languages where cliOptions are not set properly. In another PR, I will
fix the languages which I need.

On Mon, Jan 25, 2016 at 4:29 PM, Tony Tam [email protected] wrote:

This solution seems backwards. How about instead we fix the offending
language files and throw a warning when trying to set an option which has
not been configured?


Reply to this email directly or view it on GitHub
#1970 (comment)
.

evigeant and others added 2 commits January 25, 2016 21:22
Conflicts:
	modules/swagger-codegen-maven-plugin/src/main/java/io/swagger/codegen/plugin/CodeGenMojo.java
@evigeant
Copy link
Contributor Author

@fehguy I re-implemented following your suggestion. I think this is a better approach as this will allow generation with Maven to work even for languages which do not have all options defined in cliOptions.

I also added cliOptions for some languages in #1975.

@wing328
Copy link
Contributor

wing328 commented Feb 12, 2016

@evigeant If it's not too much work, I wonder if you can update indention to use 4-space instead of tab in your change as mentioned in https://google.github.io/styleguide/javaguide.html#s2.3.1-whitespace-characters

Ref: https://github.com/swagger-api/swagger-codegen/blob/master/CONTRIBUTING.md#style-guide

@wing328 wing328 added this to the v2.1.6 milestone Feb 12, 2016
@evigeant
Copy link
Contributor Author

@wing328 I fixed the indentation, even some lines which were already there.

@wing328
Copy link
Contributor

wing328 commented Feb 15, 2016

@fehguy are you happy to merge this PR into master?

fehguy added a commit that referenced this pull request Feb 16, 2016
Allow maven-codegen-plugin to set additionalProperties even if cliOptions does not define them
@fehguy fehguy merged commit e5c5dce into swagger-api:master Feb 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants