Skip to content

A quick fix to support 1 sub array like [[Object]]#3195

Open
faresd wants to merge 1 commit intoswagger-api:masterfrom
faresd:master
Open

A quick fix to support 1 sub array like [[Object]]#3195
faresd wants to merge 1 commit intoswagger-api:masterfrom
faresd:master

Conversation

@faresd
Copy link

@faresd faresd commented Jun 22, 2016

A pull request following ticket #3194, Imports don't support nested Arrays and Maps.


property.baseType = getSwaggerType(p);

// A quick fix to support 1 sub array like [[Object]]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this comment, if necessary, should be above your ~7 line addition and not above the entire block for ArrayProperty.

@wing328
Copy link
Contributor

wing328 commented Jun 23, 2016

@faresd thanks for the PR. I believe your fix only handle 1D and 2D arrays. Ideally we are looking for a recursive way to support arrays of any dimensions.

@wing328
Copy link
Contributor

wing328 commented Jun 23, 2016

@faresd I've filed #3199 to support array/map of enum for any dimensions. Here is the function with the recursive logic:

https://github.com/swagger-api/swagger-codegen/pull/3199/files#diff-0c1b6d04d7c2c41f87f7c710a5610d88R1634

Would you have cycle to revise your PR to recursively identify the inner type of array/map for import?

@wing328 wing328 added this to the v2.2.0 milestone Jun 23, 2016
@wing328 wing328 modified the milestones: v2.3.0, v2.2.0 Jul 7, 2016
@faresd
Copy link
Author

faresd commented Jul 12, 2016

@wing328 I don't think I will have the time implement the recursion.

@wing328
Copy link
Contributor

wing328 commented Jul 12, 2016

@faresd thats ok. We appreciate your contribution and will merge this PR after 2.2.0 issue if no question from anyone.

@wing328 wing328 modified the milestones: v2.2.1, v2.3.0 Aug 8, 2016
@wing328 wing328 modified the milestones: v2.3.0, v2.4.0 Dec 19, 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.

3 participants

Comments