Skip to content

Add Closure annotated Javascript Angular generator#1997

Merged
wing328 merged 6 commits intoswagger-api:masterfrom
achew22:javascript_closure
Feb 7, 2016
Merged

Add Closure annotated Javascript Angular generator#1997
wing328 merged 6 commits intoswagger-api:masterfrom
achew22:javascript_closure

Conversation

@achew22
Copy link
Contributor

@achew22 achew22 commented Jan 28, 2016

No description provided.

@wing328 wing328 added this to the v2.1.6 milestone Jan 28, 2016
@wing328
Copy link
Contributor

wing328 commented Jan 28, 2016

@achew22 thanks for the PR. Here are some feedbacks/suggestions:

  • create a shell script under ./bin/ to generate javascript-closure-angular for Petstore under samples/client/petstore/javascript-closure-angular
  • add test cases for javascript-closure-angular petstore samples. You can refer to test cases in other programming languages under the petstore folder as a reference. A good starting point is to cover GET, POST (HTTP form, JSON model), DELETE.

@achew22 achew22 force-pushed the javascript_closure branch from 13c284a to af6926b Compare January 28, 2016 06:31
@achew22
Copy link
Contributor Author

achew22 commented Jan 28, 2016

Doing the first bullet revealed something curious. Do you have any idea why it it is trying to import java.util.List?

@wing328
Copy link
Contributor

wing328 commented Jan 28, 2016

I think that's because you're using the default (Java) import mapping and would suggest you look at Go implementation for the imports: https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/GoClientCodegen.java#L94

@wing328
Copy link
Contributor

wing328 commented Jan 29, 2016

@achew22 if you need further assistance on the import mapping, please let me know.

@achew22
Copy link
Contributor Author

achew22 commented Jan 29, 2016

Funny you should mention. I managed to get the imports working for the most part. However, I'm curious how I would tell it that "binary" should be a "Blob" object? Do you have any ideas on that?

@wing328
Copy link
Contributor

wing328 commented Jan 29, 2016

Please set a typeMapping for binary, e.g.

    typeMapping.put("binary", "string");

(binary format is something we recently working on)

@wing328
Copy link
Contributor

wing328 commented Jan 29, 2016

would also be nice if you can change the indention in JavascriptClosureAngularClientCodegen.java from 2-space to 4-space.

@wing328
Copy link
Contributor

wing328 commented Jan 31, 2016

@achew22 are you still facing issue with binary and Blob?

@achew22
Copy link
Contributor Author

achew22 commented Feb 2, 2016

Sorry this took me so long to get back to you. I was traveling all weekend (and again starting tomorrow). What do you think about this?

This outputs totally valid Closure compiler compatible Javascript. There are a few cleanup things that can be done that will make the output cleaner. There might even be a few tricks that can make it compile even smaller, but I won't know till I get down into the weeds and start using this in a project. In my purely synthetic tests this was pretty good, but there is always room for improvement.

I'm going to be using these in a project I'm working on so please expect a follow-up if I find anything I can improve.

@wing328
Copy link
Contributor

wing328 commented Feb 2, 2016

@achew22 My recommendation is to release your current working version of javascript-closure-angular and document what you think can be improved so that the community can help on that as well.

@achew22
Copy link
Contributor Author

achew22 commented Feb 6, 2016

Just for reference I believe this code is good to go. I don't explicitly know what kind of optimizations are going to be available without using it in a project for real so I don't think there is anything to document. Could you please take a look at the PR?

@wing328
Copy link
Contributor

wing328 commented Feb 7, 2016

@achew22 thanks again for the contribution 🍻

PR merged into master.

@wing328 wing328 closed this Feb 7, 2016
@wing328 wing328 reopened this Feb 7, 2016
wing328 added a commit that referenced this pull request Feb 7, 2016
Add Closure annotated Javascript Angular generator
@wing328 wing328 merged commit 3174ab0 into swagger-api:master Feb 7, 2016
@achew22 achew22 deleted the javascript_closure branch February 8, 2016 03:50
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