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

[R] various bug fixes and enhancements #1965

Merged
merged 5 commits into from
Jan 26, 2019

Conversation

jokedurnez
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, 3.4.x, 4.0.x. Default: master.
  • [x ] Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

I cleaned up the mustache files to be more readable, fixed the primary types and added conditionals to responses.

Used for this specification: https://github.com/openlattice/api/blob/develop/openlattice.yaml
Thanks for the great resource !

result <- returnObject$fromJSON(httr::content(resp, "text", encoding = "UTF-8"))
Response$new(returnObject, resp)
{{/isPrimitiveType}}
{{^isPrimitiveType}}
Copy link
Member

Choose a reason for hiding this comment

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

@jokedurnez thanks for the PR. Seems like this line is missing a space in terms of alignment?

@wing328
Copy link
Member

wing328 commented Jan 23, 2019

@jokedurnez thanks for the enhancement. Can you please also run the following to update the R Petstore samples?

linux/mac: ./bin/r-petstore.sh
windows: bin\windows\r-petstore.bat

(you may notice changes not caused by this PR, which is fine)

@jokedurnez
Copy link
Contributor Author

Fixed those review comments !

@wing328 wing328 added this to the 4.0.0 milestone Jan 25, 2019
@wing328 wing328 merged commit 1a07bd6 into OpenAPITools:master Jan 26, 2019
@wing328 wing328 changed the title Bugfix/rheaders [R] various bug fixes and enhancements Jan 26, 2019
@wing328
Copy link
Member

wing328 commented Feb 10, 2019

@jokedurnez shall we use 2-space for indentation as stated in https://google.github.io/styleguide/Rguide.xml#indentation ?

@wing328
Copy link
Member

wing328 commented Feb 10, 2019

Also I've started a project to refactor the R client generator and you can track progress in https://github.com/OpenAPITools/openapi-generator/projects/8

@wing328
Copy link
Member

wing328 commented Feb 22, 2019

FYI. Filed #2215 to refactor the R client.

{{/isListContainer}}
{{/vars}}
)
gsub("[\r\n]| ", "", outstring)
Copy link
Member

Choose a reason for hiding this comment

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

I think this may cause issues if the value contains these characters

A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
* debug headers in R client

* fixes to R client

* petstore samples

* missing space

* other space :)
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.

2 participants