Skip to content

Conversation

@NicoEigenmannCW
Copy link
Contributor

PR checklist

  • [ x] Read the contribution guidelines.
  • [ x] 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\.
  • [x ] Filed the PR against the correct branch: master for non-breaking changes and 3.0.0 branch for breaking (non-backward compatible) changes.

Description of the PR

Fixes issue with ENUM generation for ruby issue #4848

@wing328
Copy link
Contributor

wing328 commented Jul 21, 2017

@wing328 wing328 merged commit b9b1b76 into swagger-api:master Jul 21, 2017
@wing328
Copy link
Contributor

wing328 commented Jul 21, 2017

@NicoEigenmannCW thanks for the contribution, which has been merged into master.

@run-o
Copy link
Contributor

run-o commented Oct 23, 2017

@wing328 @NicoEigenmannCW
Is this the desired behavior for the build_from_hash function?
It looks like this function currently checks the given value versus the enum's name rather than its actual value.
I might be missing something (and I'm a total Ruby noob), but I was under the impression that we'd want to check the value of the enum rather than the constant's name.
If we indeed care about the value, we should replace the constants.select{|c| c.to_s == value} line with constants.select{|c| {{classname}}::const_get(c) == value}

@wing328
Copy link
Contributor

wing328 commented Oct 24, 2017

@run-o I wonder if you can submit a PR so that we can review the fix more easily. (Later I'll show you where to add a test case to cover it)

@run-o
Copy link
Contributor

run-o commented Oct 24, 2017

@wing328 Yep sure, I'll work on that.

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