yahehe / Nancy.Swagger

Nancy plugin for generated API documentation in Swagger format.
MIT License
133 stars 60 forks source link

Allow RouteParamAttribute on method #157

Closed vincentparrett closed 6 years ago

vincentparrett commented 6 years ago

This allows you to document parameters even when there are no parameters at all on the handler method at all. This is needed when using only query parameters, since they don't appear on the args object you would typically extract them from the Request object inside your handler method.

This change also allows RouteParamAttribute descendants to be used - passing true to GetCustomAttributes - should not have any negative impact.

Introduces RouteParamAttribute.ParamType property to allow overrideing the param type.

yahehe commented 6 years ago

Would it be possible to get some tests for this?

vincentparrett commented 6 years ago

I have no idea how I would write tests for this? There doesn't appear to be any existing tests for the annotations stuff to use as an example.

yahehe commented 6 years ago

Yeah unfortunately our test coverage is extremely sparse, which is why I'd like to make as many tiny steps towards that goal as possible. There are no annotations tests it looks like but you could look at other tests in the solution like https://github.com/yahehe/Nancy.Swagger/blob/master/test/Nancy.Swagger.Tests/SwaggerSchemaFactoryTests.cs to see some examples. Just a couple tests validating the functionality of this change?

vincentparrett commented 6 years ago

I'm still none the wiser on how to go about testing routeparams.

jnallard commented 6 years ago

@yahehe I agree that we need tests, but I think it's unfair to hold that standard on @vincentparrett, because we didn't bother getting the tests started first.

yahehe commented 6 years ago

That's fine

jnallard commented 6 years ago

This resolves #155, by the way.

vincentparrett commented 6 years ago

Any chance you could update nuget with this PR? I can't build my project on our CI server at the moment as I don't want to check in project references.

yahehe commented 6 years ago

Hmm looks like the CI is failing, checking it out now

yahehe commented 6 years ago

2.2.51-alpha

vincentparrett commented 6 years ago

Thanks.