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] C generator refactored #2463

Merged
merged 19 commits into from Apr 15, 2019
Merged

[C] C generator refactored #2463

merged 19 commits into from Apr 15, 2019

Conversation

zhemant
Copy link
Contributor

@zhemant zhemant commented Mar 20, 2019

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, ./bin/openapi3/{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.4.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

Updated mustache for model and api to cover as much possible data types
Updated samples according to new mustaches
change data type from file to binary_t to handle binary data (support for OAS3)
Updated cmake mustache

@wing328

@auto-labeler
Copy link

auto-labeler bot commented Mar 20, 2019

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@wing328 wing328 closed this Mar 24, 2019
@wing328 wing328 reopened this Mar 24, 2019

api_response_free(respo);
fclose(file);
} else {
apiClient_free(apiClient3);
//apiClient_free(apiClient3);
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest removing this line as it's commented out and people have no idea why it's left there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated it, as apiClient object has to be freed once the requirement is over. Modified all test samples accordingly.

@wing328 wing328 added this to the 4.0.0 milestone Mar 29, 2019


// delete user test
apiClient_t *apiClient5 = apiClient_create();

UserAPI_deleteUser(apiClient5, "example123");
apiClient_free(apiClient5);
Copy link
Member

Choose a reason for hiding this comment

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

Minor suggestion: indent apiClient_free properly with reference to the previous line.

Copy link
Member

Choose a reason for hiding this comment

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

You may simply want to use the code formatter to format the manually written test files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have formatted manually written test files with uncrustify with the configuration we have in auto-generated code.

@wing328
Copy link
Member

wing328 commented Apr 4, 2019

@zhemant what about using FILE* instead?

It seems to me FILE* is pretty common for handling files in C.

@zhemant
Copy link
Contributor Author

zhemant commented Apr 8, 2019

Yes, but this new structure is keeping in mind we also need to handle binary data which may not be a file pointer. In OAS3 binary and file are the same. And in curl, we need to pass the binary/file as data(char*/uint8_t) and its size. Hence to make it common for both data type user has to pass a struct of binary_t with data and length of data so it is easy to handle both files and binary while sending.

In current codes for handling FILE* we are doing same, reading data into a struct with char* and length.

@wing328
Copy link
Member

wing328 commented Apr 15, 2019

@zhemant thanks for the explanation, which totally makes sense.

@wing328 wing328 merged commit 3be4902 into OpenAPITools:master Apr 15, 2019
@wing328
Copy link
Member

wing328 commented Apr 15, 2019

Upgrade Note

Binary (type: string, format: binary) and file (OAS v2) are now mapped to the binary_t pointer (binary_t*)

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.

None yet

2 participants