Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix C# client enum issue #774

Merged
merged 7 commits into from
Aug 11, 2018
Merged

Fix C# client enum issue #774

merged 7 commits into from
Aug 11, 2018

Conversation

wing328
Copy link
Member

@wing328 wing328 commented Aug 9, 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, 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

To fix #635

I've only run ./bin/csharp-petstore.sh for easier review (diff). I'll run other C# client scripts to update all samples later.

In addition to the fix, I've reformated the mustache templates to avoid trailing spaces and spaces in empty lines.

cc @jimschubert @mandrean @albator1932

Related PR: #737

/// <summary>
/// Enum NUMBER_MINUS_1_DOT_2 for value: -1.2
/// </summary>
[EnumMember(Value = "-1.2")]
NUMBER_MINUS_1_DOT_2 = 2
NUMBER_MINUS_1_DOT_2 = -1.2
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jimschubert @albator1932 For double/float value, do we need to restore the old behavior (using the string representation of the value)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it's not ok:

  Model\EnumTest.cs(126,30): error CS0266: Cannot implicitly convert type 'double' to 'int'. An explicit conversion exists (are you missing a cast?) [C:\projects\openapi-generator\samples\client\petstore\csharp\OpenAPIClient\src\Org.OpenAPITools\Org.OpenAPITools.csproj]
  Model\EnumTest.cs(131,36): error CS0266: Cannot implicitly convert type 'double' to 'int'. An explicit conversion exists (are you missing a cast?) [C:\projects\openapi-generator\samples\client\petstore\csharp\OpenAPIClient\src\Org.OpenAPITools\Org.OpenAPITools.csproj]

Let me fix it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed via ec5ca85

@jimschubert
Copy link
Member

@wing328 I think this looks good. I can't tell if you're done making changes, though, so I haven't merged. Can you comment when it's ready to go?

@wing328 wing328 merged commit ddb0920 into master Aug 11, 2018
@wing328 wing328 deleted the fix_csharp_enum branch August 11, 2018 03:34
@wing328
Copy link
Member Author

wing328 commented Aug 11, 2018

@jimschubert yup, thanks for the review. Just merged it.

@wing328 wing328 added this to the 3.2.1 milestone Aug 11, 2018
jimschubert added a commit to jimschubert/openapi-generator that referenced this pull request Aug 14, 2018
* master: (32 commits)
  Fixed date formatting in typescript node client (OpenAPITools#786)
  better explain usage (OpenAPITools#794)
  Fix float/double default value in C# generator (OpenAPITools#791)
  Enhancements to documentation generators (samples, default values, etc) (OpenAPITools#790)
  Remove duplicate variable declaration (OpenAPITools#792)
  Issue 758 root resource (OpenAPITools#771)
  Do not declare destructor as default when destructor is explicitly declared. (OpenAPITools#732)
  Fix C# client enum issue (OpenAPITools#774)
  [JavaScript] Update vulnerable dependencies (OpenAPITools#784)
  [Ruby] Fix method split (OpenAPITools#780)
  [Java][jaxrs-jersey] add sample with jaxrs-jersey + openapi v3 (OpenAPITools#778)
  update groupId in pom (OpenAPITools#779)
  [cpp-restsdk] Support multi-line descriptions (OpenAPITools#753)
  [Core] Resolve Inline Models (OpenAPITools#736)
  [gradle] Support nullable system property values (OpenAPITools#764)
  Correct URL for openapi-generator.cli.sh in README.md (OpenAPITools#770)
  Fixed the generation of model properties whose data type is a composed (allOf) schema (OpenAPITools#704)
  [JAX-RS][Spec] Add samples to CircleCI (OpenAPITools#759)
  minor update to python generator usage (OpenAPITools#762)
  [C++][Restbed/Pistache] Added fix for byte array (OpenAPITools#752)
  ...
@jimschubert jimschubert mentioned this pull request Aug 15, 2018
4 tasks
A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
* fix csharp enum issue

* fix numeric enum value

* add docstring to exlain isDataTypeString

* fix docstring by adding return

* fix ToJson in hash model

* remove BaseValidate for map model

* restore csproj file
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[C#] Issue with enum members
2 participants