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

[OpenAPI] Add nullable property description #3312

Closed
wants to merge 1 commit into from

Conversation

kayneth
Copy link

@kayneth kayneth commented Dec 18, 2019

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tickets -
License MIT
Doc PR api-platform/docs#...

While trying to generate a client with Jane, I encountered a missing piece of OpenAPI 3.0 for nullable properties. It's useful in order to generate models with nullable getters.

As stated on src/JsonSchema/SchemaFactory.php:173: "externalDocs is an OpenAPI specific extension, but JSON Schema allows additional keys, so we always add it" for the property schema.
Then we could also add the nullable feature.

Another take, would be to considered each components schema non required properties as optional. Therefore, I should make a PR to Jane in order to use non-required properties instead of nullable properties to generate models. But this is not a desired behaviour on their side.

@kayneth kayneth changed the title [JSONSchema] Add nullable property description [OpenAPI] Add nullable property description Dec 18, 2019
@kayneth kayneth marked this pull request as ready for review December 18, 2019 13:22
Copy link
Member

@soyuka soyuka left a comment

Choose a reason for hiding this comment

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

lgtm, may you rebase?

@kayneth
Copy link
Author

kayneth commented Dec 20, 2019

Ok, done. Thank you

Base automatically changed from master to main January 23, 2021 21:59
@alanpoulain
Copy link
Member

Done by #3402.

@alanpoulain alanpoulain closed this Mar 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants