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

Adds the ability to set root level swagger values. #153

Merged
merged 1 commit into from Jan 12, 2018

Conversation

jnallard
Copy link
Collaborator

This allows multiple values to be set on the SwaggerRoot object.

@yahehe, this seemed like the most appropriate way to do this.
I thought about giving the user the ability to just pass in a SwaggerRoot object, but I thought it was better to state which parameters will actually be set.

Fixes #147

@jnallard jnallard added this to the 2.0 milestone Jan 11, 2018
@jnallard jnallard requested a review from yahehe January 11, 2018 00:56
Description = "GitHub",
Url = "https://github.com/yahehe/Nancy.Swagger"
},
schemes: new[] { Schemes.Http }
Copy link
Owner

Choose a reason for hiding this comment

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

I assume HTTP encompasses HTTPS here?

Copy link
Owner

Choose a reason for hiding this comment

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

If it does this is good to merge, if not can we put HTTPS here instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No. Http is not the same as Https in the Schemes enum.
But when someone runs this sample, it's not going to be running on HTTPS - it defaults to HTTP.
So if they want to test the endpoints, HTTP is needed.

Copy link
Owner

Choose a reason for hiding this comment

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

I guess that's a fair point

Copy link
Owner

@yahehe yahehe left a comment

Choose a reason for hiding this comment

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

Looks good thanks dude!

@yahehe yahehe merged commit d9d95b4 into yahehe:master Jan 12, 2018
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.

None yet

2 participants