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

Allow failed validation to yield a Response instead of just a message #51

Closed
bryce-anderson opened this issue Feb 10, 2015 · 9 comments
Closed

Comments

@bryce-anderson
Copy link
Member

It may be nice to yield a 401 Unauthorized or some other type of status code. It should be easy to implement by changing the ParserResult types to either have ValidationFailure contain a Response or add a new type with the same purpose.

@kryptt
Copy link

kryptt commented Feb 13, 2015

How about suggest 403 Bad Request for the status code?

bryce-anderson added a commit that referenced this issue Feb 15, 2015
This needs some cleanup and some consideration of the naming.
@bryce-anderson
Copy link
Member Author

PR #56 attempts to address this. @kryptt, @refried, I'd appreciate any input you have.

@aryairani
Copy link

This is really great. I'm still reading it. I'm reading the tests/specs to understand, but since tests are often written in their own kind of test DSL, it would be even awesomer if you could add an example of each new feature into the com.http4s.rho.swagger.demo package too.

(I'd envision these demo services turning into tutorials that teach all of rho at some point.)

@bryce-anderson
Copy link
Member Author

@refried, Good point on the examples.

@bryce-anderson
Copy link
Member Author

@refried, do you have any more comments on #56? I'm considering merging it.

@aryairani
Copy link

No, just that I think some ticket should remain open until examples can be added too. Otherwise go for it IMO.

@bryce-anderson
Copy link
Member Author

I did add examples here and used here but perhaps that is a bit hidden. I'm going to open up a new issue category of, surprise, documentation and add a ticket for this.

Thanks for your input.

@bryce-anderson
Copy link
Member Author

This is added but not yet well documented. I'm opening a new ticket for that.

@aryairani
Copy link

Oops yeah I was looking at the wrong commit. Thanks for adding this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants