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

[Scala] extract abstract scala codegen #3755

Merged

Conversation

ePaul
Copy link
Contributor

@ePaul ePaul commented Sep 8, 2016

PR checklist

  • Read the contribution guildelines.
  • Ran the shell/batch script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates) – regenerated all scala related samples: scala, async-scala, scalatra, akka-scala)
  • Filed the PR against the correct branch: master for non-breaking changes and 2.3.0 branch for breaking (non-backward compatible) changes.

Description of the PR

This extracts a common super class for AkkaScalaClientCodegen, AsyncScalaClientCodegen, ScalaClientCodegen, ScalatraServerCodegen, which cleans up some duplicated code. This is a prerequisite to fixing #3738 (which will be done in a second pull request – #3757 – building on this one).

I tried to do it in a minimally invasive way – the results should be the same as before. In the petstore samples the only difference is that some imports for classes in the same package disappeared (only for scalatra).

I guess there could be done more, but I'm not so familiar with the Scala generators, and don't know which of the behavior differences is intentional and which not. For example, all classes seem to have different lists of reserved words.

One potential issue:

  • I extracted the languageSpecificPrimitives initialization into the super class – but it had slightly different lists in each of the subclasses. I chose to use the union of all of them, though I'm not sure what the effect would be here. (Any and Seq are now primitives for all of them, and scalatra had Integer instead of Int.)

This seems to not change anything in the output, at least for the Petstore samples.
Only one change, remove of unnecessary imports.
@wing328
Copy link
Contributor

wing328 commented Sep 11, 2016

@ePaul thanks for the PR :) I'm sure your work has laid down a more solid foundation for Scala-related generators to improve.

For languageSpecificPrimitives and reserved words, I'll create separate tasks for tracking.

@wing328 wing328 merged commit bd3a15e into swagger-api:master Sep 11, 2016
@wing328
Copy link
Contributor

wing328 commented Sep 11, 2016

Created #3770, #3771

@ePaul ePaul deleted the feature/#3738-extract-AbstractScalaCodegen branch September 11, 2016 18:16
@wing328 wing328 changed the title Feature/#3738 extract abstract scala codegen [Scala] extract abstract scala codegen Feb 20, 2017
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