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

[MRG+1] response.text #1730

Merged
merged 1 commit into from Jan 27, 2016
Merged

[MRG+1] response.text #1730

merged 1 commit into from Jan 27, 2016

Conversation

@kmike
Copy link
Member

@kmike kmike commented Jan 26, 2016

Fixes GH-1729.

TextResponse.body_as_unicode() is old and common, so it is not deprecated. I think it is better not to flood users with warnings without a compelling reason.

@codecov-io
Copy link

@codecov-io codecov-io commented Jan 26, 2016

Current coverage is 83.29%

Merging #1730 into master will not affect coverage as of 7cd020e

Powered by Codecov. Updated on successful CI builds.

@dangra
Copy link
Member

@dangra dangra commented Jan 26, 2016

I'm buying.

@dangra dangra changed the title response.text [MRG+1] response.text Jan 26, 2016
@eliasdorneles
Copy link
Member

@eliasdorneles eliasdorneles commented Jan 27, 2016

Neat! :)

eliasdorneles added a commit that referenced this pull request Jan 27, 2016
[MRG+1] response.text
@eliasdorneles eliasdorneles merged commit 7795109 into master Jan 27, 2016
2 checks passed
2 checks passed
codecov/patch 100.00% of diff hit (target 100.00%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@nramirezuy
Copy link
Contributor

@nramirezuy nramirezuy commented Jan 27, 2016

Isn't better to do it backwards ? Like body_as_unicode calling .text?
Also is not flood if you just log it once.

@kmike
Copy link
Member Author

@kmike kmike commented Jan 27, 2016

@nramirezuy yeah, you're right, it is better to call text from body_as_unicode: we'll save a function call this way.

As for deprecations - I don't have a strong opinion here, just tried to play safe in this PR to sneak it to 1.1 :)

@kmike kmike deleted the response-text branch Jan 27, 2016
@nramirezuy
Copy link
Contributor

@nramirezuy nramirezuy commented Jan 27, 2016

Just add the deprecation is fine, it wont actually flood anyone. Let's keep it clean 😄

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

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.