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] Add a supportsMixin property to codegens #3636

Closed
cbornet opened this issue Aug 24, 2016 · 2 comments
Closed

[All] Add a supportsMixin property to codegens #3636

cbornet opened this issue Aug 24, 2016 · 2 comments

Comments

@cbornet
Copy link
Contributor

cbornet commented Aug 24, 2016

  • currently if supportsInheritance is true, the properties from parent and interfaces will not be included in vars.
  • currently if supportsInheritance is true, the properties from self, parent and interfaces will all be included in allVars.
  • this works well for Javascript because it uses some "mixin" import mecanism
  • this should also work well for languages that support mixins/multi-inheritance (python, ruby, C++, ...)
  • but it doesn't work well with Java because it doesn't support mixins/multi-inheritance (java interfaces can't be used because jackson and gson need POJOs/javaBeans).

The consequence is that since we don't have the interface properties in vars, we have to duplicate the code everywhere in the template to include both vars and interfaceModels.allVars
This could be mitigated by putting some parts of the code in imported templates but that would also make the template much harder to read.
So my idea is to add a supportsMixin property for languages that can use it (eg. Javascript)

@wing328 I can do the PR if OK.

cc @demonfiddler

@demonfiddler
Copy link
Contributor

Apologies for the delay in responding – have been swamped both at work and at home.

For the first bullet I think you meant “currently if supportsInheritance is false, the properties from parent and interfaces will not be included in vars.”

I see what you mean in the Java case. It is true that Mustache templates, being logic-less, require all business logic to be implemented by the caller. If you’re confident that your change can address the requirements of JavaScript, Java and other languages whilst preserving or rectifying existing (mis-)behaviour as appropriate then I wish you good luck with the PR.

Cheers,

--A

From: Christophe Bornet [mailto:notifications@github.com]
Sent: 24 August 2016 10:38
To: swagger-api/swagger-codegen swagger-codegen@noreply.github.com
Cc: Adrian Price adrianp.quatinus@virginmedia.com; Mention mention@noreply.github.com
Subject: [swagger-api/swagger-codegen] [All] Add a supportsMixin property to codegens (#3636)

  • currently if supportsInheritance is true, the properties from parent and interfaces will not be included in vars.
  • currently if supportsInheritance is true, the properties from self, parent and interfaces will all be included in allVars/
  • this works well for Javascript because it uses some "mixin" import mecanism
  • this should also work well for languages that support mixins/multi-inheritance (python, ruby, C++, ...)
  • but it doesn't work well with Java because it doesn't support mixins/multi-inheritance (java interfaces can't be used because jackson and gson need POJOs/javaBeans).

The consequence is that since we don't have the interface properties in vars, we have to duplicate the code everywhere in the template to include both vars and interfaceModels.allVars
This could be mitigated by putting some parts of the code in imported templates but that would also make the template much harder to read.
So my idea is to add a supportsMixin property for languages that can use it (eg. Javascript)

@wing328 https://github.com/wing328 I can do the PR if OK.

cc @demonfiddler https://github.com/demonfiddler


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #3636 , or mute the thread https://github.com/notifications/unsubscribe-auth/ANXPu-LQXFPtjYdWifHJH72nzP8-49HQks5qjBDcgaJpZM4Jr0cq .

This email has been scanned by BullGuard antivirus protection.
For more info visit www.bullguard.com http://www.bullguard.com/tracking.aspx?affiliate=bullguard&buyaffiliate=smtp&url=/

@wing328
Copy link
Contributor

wing328 commented Oct 3, 2016

PR merged into master

@wing328 wing328 closed this as completed Oct 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants