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

[All] Fix some inheritance/composition issues and add allOf unit tests #3637

Merged
merged 1 commit into from
Oct 3, 2016

Conversation

cbornet
Copy link
Contributor

@cbornet cbornet commented Aug 24, 2016

Also adds a supportMixins property for languages that support mixins/multiple inheritance.

Fix #3629 #3544 #3474 #3636

@cbornet
Copy link
Contributor Author

cbornet commented Aug 25, 2016

@wing328 we are now selecting the interface that has a discriminator as parent. But for models that don't have a discriminator, should we select the first interface as parent like it was done before ?
I don't have a strong opinion on this:

  • on one hand, it allows to have some model inheritance without discriminator which some people may find useful.
  • on the other hand, if you have several interfaces, it's a bit disturbing to have different implementation for them. But I guess most of the time, models will have only one interface so that may not be a big issue.

Eventually, we could also have some "x-extends" or "x-parent" vendor extension where users could explicitely define the interface they want to be used as parent model.

@wing328 wing328 added this to the v2.2.2 milestone Sep 6, 2016
@cbornet cbornet changed the title Fix some inheritance/composition issues and add allOf unit tests [All] Fix some inheritance/composition issues and add allOf unit tests Sep 13, 2016
@AlexNolasco
Copy link
Contributor

Wondering if addProperties should account for this issue
#3829

@cacti77
Copy link

cacti77 commented Sep 30, 2016

Will this PR fix issue #3904?

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.

4 participants