-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
[Elixir] Fix generation issues and compilation warnings in Elixir generator #18788
[Elixir] Fix generation issues and compilation warnings in Elixir generator #18788
Conversation
.../openapi-generator/src/main/java/org/openapitools/codegen/languages/ElixirClientCodegen.java
Outdated
Show resolved
Hide resolved
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.
Hi @ntodd
Firstly, thank you very much for your contribution and effort on bringing the elixir openapi gen a bit more forward :)
Second, I'm not quite sure if the failing pipeline triggered an email for you, but it looks like you didn't update the sample (or may not committed them) according to the CI.
Other than that, your PR looks good to me except for the removal of the Pseudo-variables, which should be treated as reserved words as well. Thank you very much for your contribution and the effort you already put in :)!
@mrmstn I re-added the psuedo-variables and added I did not address |
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.
Hi @ntodd,
Thanks for your response and the updated code. I highly appreciated it and second, please excuse my longer response time, there's currently a lot going on, but I try me best to get the review squeezed in somewhere :)!
I looked through the generated samples, and it looks like the changes handle the primitive types not correctly for the type spec definitions!
samples/client/petstore/elixir/lib/openapi_petstore/api/fake.ex
Outdated
Show resolved
Hide resolved
samples/client/petstore/elixir/lib/openapi_petstore/api/fake.ex
Outdated
Show resolved
Hide resolved
samples/client/petstore/elixir/lib/openapi_petstore/api/fake.ex
Outdated
Show resolved
Hide resolved
samples/client/petstore/elixir/lib/openapi_petstore/api/fake.ex
Outdated
Show resolved
Hide resolved
samples/client/petstore/elixir/lib/openapi_petstore/api/fake.ex
Outdated
Show resolved
Hide resolved
samples/client/petstore/elixir/lib/openapi_petstore/api/fake.ex
Outdated
Show resolved
Hide resolved
samples/client/petstore/elixir/lib/openapi_petstore/api/fake.ex
Outdated
Show resolved
Hide resolved
samples/client/petstore/elixir/lib/openapi_petstore/api/user.ex
Outdated
Show resolved
Hide resolved
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.
Hi @ntodd,
I've made some changes to your code to get this PR moving forward. In my opinion, everything should be correct, but it would be great if you could review my changes to ensure I haven't missed anything.
If the changes look good to you, let's go ahead and merge this. If I don't hear back from you, I'll assume the changes are fine and plan to merge them by Tuesday.
The changes will be included in release 7.7.0. Alternatively, if you prefer to use the latest features, you can access them in the latest Docker container after the merge.
Thanks again for your time and effort! Your contributions are highly appreciated!
@mrmstn Thanks for catching what I missed. Looks good to me! |
This PR improves Elixir code generation by resolving several issues observed when generating using the Procore API spec:
@mrmstn
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master
(upcoming 7.6.0 minor release - breaking changes with fallbacks),8.0.x
(breaking changes without fallbacks)