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

Update README.md #1652

Merged
merged 2 commits into from Jun 23, 2015
Merged

Update README.md #1652

merged 2 commits into from Jun 23, 2015

Conversation

daniel347x
Copy link
Contributor

The documentation for the 'encoding : null' argument does not emphasize that this is necessary for binary response data, something that is not obvious - see http://stackoverflow.com/a/14855016/368896. I've added a parenthetical comment to emphasize this in the documentation. This would have saved me (and hopefully will save others).

The documentation for the 'encoding : null' argument does not emphasize that this is necessary for binary response data.  See http://stackoverflow.com/a/14855016/368896.  I've added a parenthetical comment to emphasize this in the documentation.
@simov
Copy link
Member

simov commented Jun 23, 2015

Yep, I agree that encoding:null is a too clever option. However I would like the wording to be a bit more declarative, and especially I don't like the Warning bits. How about something a bit shorter and less noisy?

Note: if you expect binary data, you should set encoding: null

Binary data can be anything so mentioning PDF and images is useless. In fact this option is related to the next one as well (gzip), in case that you want to receive the gzipped content and handle it by yourself.

Consolidated warning about binary data and `encoding: null`.
@daniel347x
Copy link
Contributor Author

That's good! I've updated it to match. I did leave it in parentheses, to give the sense that this is a 'heads up'. The note would have saved me!

simov added a commit that referenced this pull request Jun 23, 2015
Update `encoding` option documentation in README.md
@simov simov merged commit f8ae6d4 into request:master Jun 23, 2015
@simov
Copy link
Member

simov commented Jun 23, 2015

Thanks 👍

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.

None yet

2 participants