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

Setup an interface for writing http responses based on the Content-Type header #6815

Merged
merged 1 commit into from
Jul 29, 2016

Conversation

jsternberg
Copy link
Contributor

Only application/json is supported right now, but this opens up the
easier possibility of additional content types to be returned from the
server.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @e-dard, @gunnaraasen and @joelegasse to be potential reviewers

@jsternberg jsternberg force-pushed the js-content-type-response-writer branch from 1fd2e34 to ae2bb35 Compare June 9, 2016 02:47
@jsternberg
Copy link
Contributor Author

There's still more work involved with completely integrating this. Errors are still always sent back as JSON instead of the proper content type.

@jsternberg jsternberg force-pushed the js-content-type-response-writer branch 2 times, most recently from 677fd1f to 1b06ec9 Compare June 9, 2016 15:52
@jsternberg
Copy link
Contributor Author

I think this should work correctly now. Now other handlers, like authentication, go through the custom ResponseWriter that controls formatting the output. The package continues to pass golint.

The only caveat is that a minor change has been made to the output that shouldn't matter, but is worth noting. Queries without chunking previously would not end in a newline. Now queries without chunking end in a newline to be consistent with everybody else. That adds one extra byte to be returned. If we need to keep the old behavior, it's possible, but requires much hackier code.

@jsternberg jsternberg force-pushed the js-content-type-response-writer branch 2 times, most recently from c9aa644 to d2d4cba Compare June 10, 2016 19:58
@corylanou
Copy link
Contributor

+1 looks good.

…pe header

Only `application/json` is supported right now, but this opens up the
easier possibility of additional content types to be returned from the
server.
@jsternberg jsternberg force-pushed the js-content-type-response-writer branch from d2d4cba to d85072a Compare July 29, 2016 18:15
@jsternberg jsternberg merged commit 4a89ab4 into master Jul 29, 2016
@mark-rushakoff mark-rushakoff deleted the js-content-type-response-writer branch August 24, 2016 23:05
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.

3 participants