Skip to content

Conversation

@RichiWIP
Copy link
Contributor

@RichiWIP RichiWIP commented Jan 11, 2017

Description of the PR

Arrays are now serialized according to the different collection formats.
For collections of format "multi", all items are now appended to the parameter.
For all other formats, a string is constructed using Array.join(). The delimiter for this is read from a constant map exported by variables.ts.
All api classes are also exported in a const array to make handling of large api libraries easier.

#4531

Richard Naeve and others added 2 commits January 11, 2017 11:24
Arrays are now serialized according to the different collection formats. All api classes are also exported in a const array to make handling of large api libraries easier.
}

headers.set('api_key', String(apiKey));
headers.set('api_key', String(apiKey));
Copy link
Contributor

Choose a reason for hiding this comment

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

Several additional spaces added to this line. Not sure if it's related to this PR.

let headers = new Headers(this.defaultHeaders.toJSON()); // https://github.com/angular/angular/issues/6845
if (status !== undefined) {
queryParameters.set('status', <any>status);
if(status){
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do if (status) { instead so as to conform to the TS style guide: https://github.com/Microsoft/TypeScript/wiki/Coding-guidelines#style ?

{{#isListContainer}}
if({{paramName}}){
{{#isCollectionFormatMulti}}
{{paramName}}.forEach((element) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

There seems to be 4 additional spaces added to the indentation.

@wing328
Copy link
Contributor

wing328 commented Jan 11, 2017

@RichiWIP thanks for the PR. I've left some comments.

For if statement, I prefer something like the following

if (status) {
  • a space before and after condition
  • open curly bracket in the same line

Thanks for removing the trailing spaces in the templates.

Ref: https://github.com/Microsoft/TypeScript/wiki/Coding-guidelines#style

@wing328 wing328 merged commit 2e7e258 into swagger-api:master Jan 13, 2017
@wing328
Copy link
Contributor

wing328 commented Jan 13, 2017

@RichiWIP thanks for your PR, which looks good to me and has been merged into master.

cc @Vrolijkx

@wing328 wing328 changed the title Issue 4531 [TS][Angular2] better support for collection formats Feb 20, 2017
@kernwig
Copy link
Contributor

kernwig commented Mar 14, 2017

This change has been lost in the v2.3.0 branch.

@wing328
Copy link
Contributor

wing328 commented Mar 16, 2017

@kernwig let me cherry-pick the commits into 2.3.0 and see how it goes...

@wing328
Copy link
Contributor

wing328 commented Mar 18, 2017

@kernwig I've manually reviewed and applied the fix (1 liner) via #5109. Please take a look when you've time.

@kernwig
Copy link
Contributor

kernwig commented Mar 20, 2017

@wing328 I added that line and the project compiles again. I don't know see what purpose it serves though. I believe the APIS variable was originally useful in v2.2 to import and provide the services in our own modules. In v2.3, the template creates api.module.ts, which imports each service. Thus the APIS constant is now unused.

@kernwig
Copy link
Contributor

kernwig commented Mar 20, 2017

Oops. No, my IDE was happy but failed at compile time. You used class name instead of file name.

Correct template line is:

import { {{ classname }} } from './{{ apiFilename }}';

@wing328
Copy link
Contributor

wing328 commented Mar 21, 2017

@kernwig right, I'll file a PR to use {{apiFilename}} in both master and 2.3.0

@wing328
Copy link
Contributor

wing328 commented Mar 21, 2017

@kernwig I've filed #5131 to fix the API file using {{classFilename}}. Please take a look to see if the change looks good to you.

I also update the line about export. Please review as well.

davidgri pushed a commit to davidgri/swagger-codegen that referenced this pull request May 11, 2017
* ISSUE-4531
Arrays are now serialized according to the different collection formats. All api classes are also exported in a const array to make handling of large api libraries easier.

* Added petstore samples

* Fixed indentations and coding style
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.

3 participants