Skip to content
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

[C#] Fix issue #1088 with generation of enum classes referenced from other objects #1089

Merged
merged 3 commits into from
Oct 11, 2018

Conversation

rubms
Copy link
Contributor

@rubms rubms commented Sep 24, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 3.3.x, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Fixed the #1088 issue. As explained in the suggestion in the issue description, I removed the update of enum properties, in AbstractCSharpCodeGen.java, for enums referenced from other objects in the OpenAPI specification. The removed update was incorrectly setting the isString properties of enum values to false, resulting in an incorrect generation of the enum class, while it is apparently providing no benefit at all.

c.c. C# technical committee: @mandrean (2017/08) @jimschubert (2017/09)

@jimschubert
Copy link
Member

Sorry for the late response on this… I'm in the middle of purchasing a house and it's taking a lot out of me 💃

I ran this locally and it worked as expected. Seems like something must have been changed in updateCodegenPropertyEnum since the original work (I think I had originally added this to fix referenced enums about a year ago).

@jimschubert jimschubert merged commit 529a638 into OpenAPITools:master Oct 11, 2018
@jimschubert jimschubert self-assigned this Oct 11, 2018
@rubms rubms deleted the Fix_issue_#1088 branch October 16, 2018 07:44
@wing328 wing328 added this to the 3.3.3 milestone Oct 31, 2018
@wing328
Copy link
Member

wing328 commented Nov 15, 2018

@rubms thanks for the PR, which has been included in the v3.3.3 release: https://twitter.com/oas_generator/status/1062929948191510528

A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
…renced from other objects (OpenAPITools#1089)

* Run ./bin/utils/ensure-up-to-date to re-generate samples run in the CI.

* Fixed the OpenAPITools#1088 issue by removing the update of enumeration properties while processing objects that reference them. Launched the ./bin/csharp-petstore-all.sh script.
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