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

Add examples #62

Merged
merged 1 commit into from Nov 20, 2015
Merged

Add examples #62

merged 1 commit into from Nov 20, 2015

Conversation

sb8244
Copy link
Contributor

@sb8244 sb8244 commented Nov 19, 2015

This adds support for the example property on swagger 1.2 models and model properties. They will map over the the identical field in swagger 2.0 definitions.

#61

properties[propertyName] = extend({},
this.buildDataType(oldProperty, true),
{description: oldProperty.description}
allowedProperties
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need such big changes.
Just add example: undefinedIfEmpty(oldProperty.example) in two places
Here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will that make example key be present even if it were undefined?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, only if it proper value.
For example it also skip null and empty string

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That will put an undefined value for the example key, no?

On Thursday, November 19, 2015, Ivan Goncharov notifications@github.com
wrote:

In index.js
#62 (comment)
:

 properties[propertyName] = extend({},
   this.buildDataType(oldProperty, true),
  •  {description: oldProperty.description}
    
  •  allowedProperties
    

You can see check here
https://github.com/apigee-127/swagger-converter/blob/master/index.js#L824


Reply to this email directly or view it on GitHub
https://github.com/apigee-127/swagger-converter/pull/62/files#r45416240.

Steve Bussey •SalesLoft

•Software Engineer

e steve.bussey@salesloft.com w salesloft.com
http://salesloft.com/?v=1&utm_expid=76406993-1.E5ncJhwyTAW9uGaR1WdBTA.1?utm_source=Email-Signature-Rescue&utm_medium=Email-Signature&utm_campaign=Email-Signature-Rescue

a 3423 Piedmont Rd NE, Atlanta, GA 30305

[image: Facebook] https://www.facebook.com/SalesLoft [image: Twitter]
https://twitter.com/SalesLoft [image: Linkedin]
https://www.linkedin.com/company/salesloft [image: wordpress]
http://blog.salesloft.com/blog/ [image:
http://salesloft.com/prospector/#video]
http://salesloft.com/prospector/#video [image: Software Engineer Logo]
http://salesloft.com/?v=1&utm_expid=76406993-1.E5ncJhwyTAW9uGaR1WdBTA.1?utm_source=Email-Signature-Rescue&utm_medium=Email-Signature&utm_campaign=Email-Signature-Rescue

Copy link
Collaborator

Choose a reason for hiding this comment

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

@sb8244 Sorry I made mistake. You don't need undefinedIfEmpty wrapper.
simply example: oldProperty.example.

Copy link
Collaborator

Choose a reason for hiding this comment

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

extend function checks values internally, see: https://github.com/apigee-127/swagger-converter/blob/master/index.js#L798

Copy link
Collaborator

Choose a reason for hiding this comment

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

We convert a lot of optional fields so to check each of them individually mean huge code bloat.
At least 3 lines instead of single one.

@mohsen1
Copy link
Contributor

mohsen1 commented Nov 19, 2015

@IvanGoncharov please review and let me know when it's good to merge.

}
}
},
"parameters": [{
Copy link
Collaborator

Choose a reason for hiding this comment

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

@sb8244
You have custom formatting of ouput file so it require manual fixing each time we change something in the lib.
Please uncomment this lines run test and copy complex-models.json-converted file.

@mohsen1 What do you think about doing it similar to this?

@IvanGoncharov
Copy link
Collaborator

@sb8244 Also can you also squash your changes into one commit, it will simplify history.
Not mandatory, but would be nice if you split code formatting(removing empty line, ...) into separate commit.

@sb8244
Copy link
Contributor Author

sb8244 commented Nov 20, 2015

I would be happy to squash

Example property under swagger 1.2 model is kept in the swagger 2.0 definition
If a swagger 1.2 model has the example key, it will be transferred to the swagger 2.0 definition
@sb8244
Copy link
Contributor Author

sb8244 commented Nov 20, 2015

@IvanGoncharov I have squashed & fixed the json formatting

@IvanGoncharov
Copy link
Collaborator

@sb8244 Thanks
@mohsen1 👍 ready for merge

mohsen1 added a commit that referenced this pull request Nov 20, 2015
@mohsen1 mohsen1 merged commit 4be1712 into apigee-127:master Nov 20, 2015
@mohsen1
Copy link
Contributor

mohsen1 commented Nov 20, 2015

Thank you! 😀

@sb8244 sb8244 deleted the add-examples branch November 20, 2015 01:18
@sb8244
Copy link
Contributor Author

sb8244 commented Nov 20, 2015

Thank you both! I'll PR again if I find any weirdness.

Flow right now is swagger-docs ruby bindings to swagger 1.2 json files, then running a conversion script to go from 1.2 to 2.0. From there, pipe it into a themed swagger-ui.

@IvanGoncharov
Copy link
Collaborator

@sb8244 SwaggerUI has native support for 1.2(it has its own converter).
Did you try that?
If so, what was your experience with it?

@sb8244
Copy link
Contributor Author

sb8244 commented Nov 20, 2015

I have been using https://github.com/jensoleg/swagger-ui which is a much more production ready documentation visual than the default theme. This theme does support 1.2, but the examples section isn't processed unless it's in 2.0, because of the change from model to definitions.

whitlockjc pushed a commit that referenced this pull request Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants