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

Move to public the `SwaggerDocument` object of the router. #1483

Merged
merged 4 commits into from Sep 17, 2019

Conversation

@mbarnach
Copy link
Contributor

mbarnach commented Aug 29, 2019

Add title, description and version to the constructer in order to be specified by the user.
Add a related test.

Description

The swagger object inside Router is now public.
The constructor of SwaggerDocument has been extended to have the title, description and version fields with the previous values as default.

Motivation and Context

When generating the Swagger document, the meta data like title, description and version couldn't be changed.

Fix issue #1482.

How Has This Been Tested?

A new test has been added.
It won't affect existing code, as it is a purely adding feature.

Checklist:

  • If applicable, I have updated the documentation accordingly.
  • If applicable, I have added tests to cover my changes.
Add `title`, `description` and `version` to the constructer in order to be specified by the user.
Add a related test.
@djones6 djones6 self-requested a review Sep 9, 2019
@@ -120,7 +120,7 @@ public class Router {
// MARK: Swagger
/// Contains the structures needed for swagger document generation
var swagger: SwaggerDocument
public var swagger: SwaggerDocument

This comment has been minimized.

Copy link
@djones6

djones6 Sep 9, 2019

Member

The problem with this approach is that it introduces an ordering problem. Routes are added to the swagger document each time you call router.get() et. al, and so if you were to reassign router.swagger after registering some routes, those routes would no longer appear in the document.

Perhaps you could leave this internal, and instead add an additional parameter eg. apiDocument: SwaggerDocument = SwaggerDocument() to router.init?

…o the router initialiser.
Sources/Kitura/Router.swift Show resolved Hide resolved
@@ -120,7 +120,7 @@ public class Router {
// MARK: Swagger
/// Contains the structures needed for swagger document generation
var swagger: SwaggerDocument
internal var swagger: SwaggerDocument

This comment has been minimized.

Copy link
@djones6

djones6 Sep 10, 2019

Member

internal is implicit, you can remove it.

Removing implicit `internal` qualifier.
@mbarnach

This comment has been minimized.

Copy link
Contributor Author

mbarnach commented Sep 10, 2019

I think the failure in Travis is not related to the PR. Can we re-run the tests?

@djones6 djones6 merged commit 0bab2d2 into IBM-Swift:master Sep 17, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.