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

Feature/add octet stream content type #745

Merged

Conversation

akabraham
Copy link
Contributor

No description provided.

@dblock
Copy link
Member

dblock commented Aug 29, 2014

I think this content type is frequent enough to be added to Grape proper, however it needs tests and to be documented to be merged. Also please squash all your commits.

@akabraham akabraham force-pushed the feature/add-octet-stream-content-type branch 3 times, most recently from 3d2c153 to 2052ccb Compare August 29, 2014 22:20
@akabraham
Copy link
Contributor Author

Ok I've added a test for application/octet-stream along with tests for rss, atom, and jsonapi (other untested content-types). Also updated the README. Let me know if this works

@@ -2,6 +2,7 @@
============

* Your contribution here.
* [#745](https://github.com/intridea/grape/pull/745): Added `:binary, application/octet-stream` content-type - [@contributor](https://github.com/akabraham).
Copy link
Member

Choose a reason for hiding this comment

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

You're not @contrbutor :) Change to @akabraham.

Updated request_response_spec test for new content_type

Updated CHANGELOG

Added tests for application/octet-stream and other untested content-types

Updated README for undocumented content-types. Flipped :binary and :txt order in content_types.rb

Removed atom, rss, and jsonapi content-types since they were never properly supported. Updated test for binary content-type.
@akabraham akabraham force-pushed the feature/add-octet-stream-content-type branch from 2052ccb to 41bd66d Compare September 3, 2014 03:46
@akabraham
Copy link
Contributor Author

Ok made the changes. Let me know if this is good

@dblock
Copy link
Member

dblock commented Sep 3, 2014

Perfect, merging.

dblock added a commit that referenced this pull request Sep 3, 2014
…nt-type

Feature/add octet stream content type
@dblock dblock merged commit 4566111 into ruby-grape:master Sep 3, 2014
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.

2 participants